Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Puma to dependencies #214

Merged
merged 1 commit into from
Oct 26, 2021
Merged

Add Puma to dependencies #214

merged 1 commit into from
Oct 26, 2021

Conversation

karlbaker02
Copy link
Contributor

@karlbaker02 karlbaker02 commented Oct 20, 2021

This PR adds Puma to govuk_app_template's dependencies, along with a config file defining sensible defaults (and trying to keep inline with Unicorn to a large degree). Puma will be used as the default webserver when apps are replatformed to Kubernetes; this commit enables that work to progress using a single source of truth for configuration and following the same practice as we currently use for managing Unicorn.

Trello card

Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey 👋 no idea if you're ready for a review on this or not, apologies if you aren't. I saw it prop up so thought I'd add a couple of quick thoughts, do let me know if I've been a pain.

govuk_app_config.gemspec Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
@karlbaker02
Copy link
Contributor Author

Hey 👋 no idea if you're ready for a review on this or not, apologies if you aren't. I saw it prop up so thought I'd add a couple of quick thoughts, do let me know if I've been a pain.

Thanks Kevin, very useful and definitely not a pain!

Copy link
Contributor

@sengi sengi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I added a bunch of suggested changes to trim the verbose comments. (I know they're just a straight move from the original but thought might as well clean them up so they're not re-stating what's already clear from reading the code.)

lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
lib/govuk_app_config/govuk_puma.rb Outdated Show resolved Hide resolved
This commit adds Puma to govuk_app_template's dependencies, along with a config file defining sensible defaults (and trying to keep inline with Unicorn to a large degree). Puma will be used as the default webserver when apps are replatformed to Kubernetes; this commit enables that work to progress using a single source of truth for configuration and following the same practice as we currently use for managing Unicorn.
@karlbaker02 karlbaker02 merged commit c56da78 into main Oct 26, 2021
@karlbaker02 karlbaker02 deleted the add-puma branch October 26, 2021 15:02
karlbaker02 pushed a commit that referenced this pull request Oct 26, 2021
Releases:

- Add Puma to dependencies ([#214](#214))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants