phoenixframework / phoenix

Peace of mind from prototype to production
https://www.phoenixframework.org
MIT License
21.45k stars 2.88k forks source link

docs: router, fix scope example explanation typo #5929

Closed jordannb closed 2 months ago

jordannb commented 2 months ago

This is my first PR, as I was reading through docs as someone new to Phoenix framework and noticed this section of the Phoenix.Router docs didn't make sense to me.

This fixes a typo where a host option passed to scope/2 is described as if it were a part of the path param instead. I updated the description instead of fixing the example, because I personally found the example to be useful in better understanding the options available when defining router scopes, but changing the example instead would also work if preferred.

I removed the sentence about named routes because:

  1. They aren't explained previously in this documentation.
  2. They seem to be related to helpers (deprecated), as the phx.routes mix task doesn't output them when helpers are disabled, so it's not clear to me if it's a useful detail to include.

If it's better to keep that sentence, I believe the named route provided is incorrect, since the host value isn't included in the named routes (based only on own local testing). It also could be helpful to link to an explanation of named routes in this case, but I didn't find a good reference for this and don't have the expertise to write it.

Gazler commented 2 months ago

It looks like this was accidentally changed in this commit:

https://github.com/phoenixframework/phoenix/commit/16f2ef38057c12f68636bfe9350340a165785b68#diff-c474801b0de930e7c3fd5808258094655afe8b008149ff17bee4caaf0d85a154L177

The previous version of the documentation was:

https://hexdocs.pm/phoenix/1.6.16/Phoenix.Router.html#module-scopes-and-resources

I think we should go back to this version for now, whilst we still support route helpers.

jordannb commented 2 months ago

It looks like this was accidentally changed in this commit:

16f2ef3#diff-c474801b0de930e7c3fd5808258094655afe8b008149ff17bee4caaf0d85a154L177

The previous version of the documentation was:

https://hexdocs.pm/phoenix/1.6.16/Phoenix.Router.html#module-scopes-and-resources

I think we should go back to this version for now, whilst we still support route helpers.

@Gazler Thank you. I updated the PR to revert the example based on the PR you shared. Other minor changes are included that I believe a) improve consistency and b) correct the documented arity of the scope macro called.

Gazler commented 2 months ago

Perfect. Thanks <3