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 --adapter flag #5071

Merged
merged 6 commits into from
Aug 2, 2023
Merged

Conversation

moogle19
Copy link
Contributor

@moogle19 moogle19 commented Nov 4, 2022

Adds a --adapter flag to mix phx.new to choose the web adapter for a new application (allows cowboy or bandit, defaults to cowboy).

I am not quite happy with having the versions in the generator.ex, but not sure if using >= 0.0.0 (like for the ecto_adapters) will lead to problems in the future.

@Gazler
Copy link
Member

Gazler commented Nov 4, 2022

I'm not sure if we're ready to merge this yet, but it's worth updating the following files too:

https://github.com/phoenixframework/phoenix/blob/master/installer/templates/phx_umbrella/apps/app_name_web/config/config.exs
https://github.com/phoenixframework/phoenix/blob/master/installer/templates/phx_umbrella/apps/app_name_web/mix.exs

@mtrudel
Copy link
Member

mtrudel commented Nov 5, 2022

I was planning on submitting a PR for this after 1.7 was out, on the reasoning that we should probably let bandit support bake in for a bit in an 'unofficial' capacity before having Phoenix put any sort of 'official' stamp on it (There's also some documentation changes & other minor stuff that's in a similar boat). This feels like a 1.7.x type thing, lest Bandit support get a bad name if anything serious comes up during the inevitable teething process.

When the time does come, this PR looks great though!

@mtrudel
Copy link
Member

mtrudel commented Nov 5, 2022

There's also test coverage to be added here - feel free to take at will from the (mostly done, but not heavily tested) copy of the work I've done on this! https://github.com/mtrudel/phoenix/tree/bandit_generator

@mtrudel
Copy link
Member

mtrudel commented Nov 7, 2022

Looks great! Maybe we can revisit inclusion with 1.7.x (for some small x)?

@mtrudel
Copy link
Member

mtrudel commented Jan 1, 2023

This continues to look good to me; I'll leave it at the discretion of @josevalim et al about timing, but from my perspective this is ready to go anytime.

@chrismccord chrismccord self-assigned this Jan 2, 2023
@chrismccord chrismccord merged commit 0ed0821 into phoenixframework:main Aug 2, 2023
4 checks passed
@chrismccord
Copy link
Member

❤️❤️❤️🐥🔥

@mtrudel
Copy link
Member

mtrudel commented Aug 2, 2023

Woooo!

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.

5 participants