Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Book][Routing] Add example about how to match multiple methods#5201

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
weaverryan merged 1 commit intosymfony:2.3fromxelaris:routing-multi-method
Jul 16, 2015

Conversation

@xelaris
Copy link
Contributor

QA
Doc fix?no
New docs?yes
Applies to2.3+
Fixed tickets

@weaverryan
Copy link
Member

I like this, but I'd actually like to solve it in an easier way: add a commented-out example in the previous section:

contact_process:path:/contactdefaults:{ _controller: AppBundle:Main:processContact }methods:[POST]# or to match GET *or* POST methods# methods: [GET, POST]

What do you think@xelaris?

@javiereguiluz
Copy link
Member

I usually prefer concise explanations, as proposed by@weaverryan. But in this case, I think it could be better to show a verbose explanation as proposed by@xelaris. Adding a comment is not clear enough for me. Let's ask@xabbuh and@wouterj for their opinion about this. Thanks.

@xabbuh
Copy link
Member

I would have said that the example proposed by@weaverryan would be clear enough.

@javiereguiluz Do you think from your experience about talking with beginners that this is something not so obvious (even with commented out example)?

@javiereguiluz
Copy link
Member

@xabbuh I don't know which would be the opinion of newcomers. It was just a personal comment. Let me show you a pair of screenshots explainig why I said that.

We have this:

before1

And Ryan proposes to add a comment:

after1

It's not a bad idea at all. But my concern is ... what would happen with the other 3 config formats? For example, consider the case of "annotations":

Before:

before2

After:

after2

But I think we could find a solution: what if we useGET forcontactAction () andGET +POST forprocessContactAction()?

@weaverryan
Copy link
Member

I hadn't thought about the annotations format, so I agree with Javier. But making the process handle get or post might be confusing since it has the same URL as the above, and so will never match in GET. I'm in favor of actually merging as is, after making all this fuss (the additions actually small - mostly code block).

@wouterj
Copy link
Member

What about using[POST, PUT] (for instance) for processContactForm?

@weaverryan
Copy link
Member

Oh, that's a cool idea - I can't believe I didn't think of that! +1 for making the previous example use Post and Put instead of this new section.@xelaris can you make that change?

Thanks!

@xabbuh
Copy link
Member

Oh yeah, that's indeed a great idea@wouterj.

@xelarisxelarisforce-pushed therouting-multi-method branch from6f854f9 to29dbb8dCompareJuly 5, 2015 19:08
@xelaris
Copy link
ContributorAuthor

I like the idea of changing the existing example to demonstrate how to match multiple methods and I changed the PR as suggested by@wouterj. But I have two points about the current state of this PR:

  • I noticed that the example doesn't work well with the best practices, as it uses two actions to display and process the form. This was actually a reason for the first version of this PR.
  • I'm in doubt ifPUT is a suitable method to send a contact form.

I'm +1 with@javiereguiluz suggestion to change theprocessContactAction() method to matchGET andPOST. This would kill two birds with one stone: Demonstrate how to match multiple routes and fulfill the best practise.
To make the example more consistent then, I suggest to renameprocessContactAction tocontactFormAction and use a comment like// ... display and process a contact form and to changecontactAction tonewsAction with route/news or something and use a comment like// ... display your news.

What are your opinion about this?

@xabbuh
Copy link
Member

@xelaris Good point with the best practices. But wouldn't we then don't have an example with only one allowed method when choosing your solution?

@xelaris
Copy link
ContributorAuthor

@xabbuh ThenewsAction with route/newswould match onlyGET.

@xabbuh
Copy link
Member

Good point, looks like a good idea then.

@xelaris
Copy link
ContributorAuthor

I have changed the PR to apply the latest proposal. What do you think about it?

@xabbuh
Copy link
Member

👍 from me

@weaverryan
Copy link
Member

LOVE it. Thanks@xelaris - you rock for your patience and ideas!

@weaverryanweaverryan merged commitc1e8453 intosymfony:2.3Jul 16, 2015
weaverryan added a commit that referenced this pull requestJul 16, 2015
… methods (xelaris)This PR was merged into the 2.3 branch.Discussion----------[Book][Routing] Add example about how to match multiple methods| Q             | A| ------------- | ---| Doc fix?      | no| New docs?     | yes| Applies to    | 2.3+| Fixed tickets |Commits-------c1e8453 [Book][Routing] Change example to match multiple methods
@xelarisxelaris deleted the routing-multi-method branchOctober 31, 2022 12:17
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xelaris@weaverryan@javiereguiluz@xabbuh@wouterj

[8]ページ先頭

©2009-2025 Movatter.jp