Skip to content

Add --adapter flag#5071

Merged
chrismccord merged 6 commits into
phoenixframework:mainfrom
moogle19:bandit_support
Aug 2, 2023
Merged

Add --adapter flag#5071
chrismccord merged 6 commits into
phoenixframework:mainfrom
moogle19:bandit_support

Conversation

@moogle19
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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!

Comment thread installer/templates/phx_single/mix.exs Outdated
@mtrudel
Copy link
Copy Markdown
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
Copy Markdown
Member

mtrudel commented Nov 7, 2022

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

@mtrudel
Copy link
Copy Markdown
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
@chrismccord
Copy link
Copy Markdown
Member

❤️❤️❤️🐥🔥

@mtrudel
Copy link
Copy Markdown
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