Skip to content

Fixes wrong tag for RouteLoaderInterface registration without autoconfiguration #13474

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

Closed
wants to merge 1 commit into from

Conversation

pounard
Copy link
Contributor

@pounard pounard commented Apr 1, 2020

I had to do a few greps to find the right tag to use, without the tag, it was working seamlessly with 4.4., but since 5.0. I had to add it.

That aside, I'm using this PR to say thank you for this page of documentation, aside this minor typo it helped me a lot. 👍

@fancyweb
Copy link
Contributor

fancyweb commented Apr 1, 2020

👍
#12845 was a mistake
routing.route_loader is the tag to use for service route loaders.
routing.loader is another tag to mark routes "config" loaders.

However, it should be done on 4.4, isn'it?

@pounard
Copy link
Contributor Author

pounard commented Apr 1, 2020

It probably should, but I experienced a crash while migrating from 4.4 to 5.0, which means that with 4.4 you don't even need the tag, it just works. But it would be wise to add it anyway.

@fancyweb
Copy link
Contributor

fancyweb commented Apr 1, 2020

Yes you don't need it on 4.4. Not tagging on 4.4 is deprecated. Then it is required on 5.0.

But the tag is wrong on 4.4 doc too. That's why you need to make the change on 4.4. Then it will be also modified on 5.0 when the branch will be merged up.

@pounard
Copy link
Contributor Author

pounard commented Apr 2, 2020

OK, I'll do it.

@pounard
Copy link
Contributor Author

pounard commented Apr 2, 2020

@fancyweb #13478

I didn't know that each Symfony version was a branch, I'm discovering how to contribute to documentation, Please tell me if I did something wrong.

@fancyweb
Copy link
Contributor

fancyweb commented Apr 2, 2020

#13478 is good. You can close this one. Thanks.

@pounard
Copy link
Contributor Author

pounard commented Apr 2, 2020

OK ! Thanks.

@pounard pounard closed this Apr 2, 2020
@xabbuh xabbuh added the Routing label Apr 2, 2020
@javiereguiluz
Copy link
Member

@pounard thanks for your contribution! We know that at first it's hard to know the exact branch where something needs to be fixed. But you don't have to worry, because maintainers can help you find the right branch. Also, we can change the branch automatically while merging pull requests, so next time you don't have to close a pull request and create a new one. That will save you some time. Thanks!

javiereguiluz added a commit that referenced this pull request Apr 2, 2020
…thout autoconfiguration (pounard)

This PR was merged into the 4.4 branch.

Discussion
----------

Fixes wrong tag for RouteLoaderInterface registration without autoconfiguration

Following #13474

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

Commits
-------

13a92be Fixes wrong tag for RouteLoaderInterface registration without autoconfiguration
@fancyweb
Copy link
Contributor

fancyweb commented Apr 2, 2020

Also, we can change the branch automatically while merging pull requests, so next time you don't have to close a pull request and create a new one.

I learned something too 😁

@pounard
Copy link
Contributor Author

pounard commented Apr 2, 2020

But you don't have to worry, because maintainers can help you find the right branch. Also, we can change the branch automatically while merging pull requests, so next time you don't have to close a pull request and create a new one. That will save you some time.

OK, being an external contributor, I don't see all github options so I wasn't aware of that. Thanks for the explanation !

Once again, thank you very much for all the documentation, even thought it's not always enough for me, being a developer I often get the obvious before reading the doc by debugging the code first, it's still a very good documentation and I very much appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants