Re-adding the ability to add a resource to the RouteCollectionBuilder #16361
Conversation
418ebdb to
549a3e2
Compare
| * | ||
| * @return $this | ||
| */ | ||
| private function addResource(ResourceInterface $resource) |
There was a problem hiding this comment.
private methods should come after public ones.
There was a problem hiding this comment.
Actually, shouldn't this method be public? Otherwise I fail to see how $resources should ever be changed.
There was a problem hiding this comment.
It can be private. As odd as it sees, since this method is being called from inside RouteCollectionBuilder (even though it's not being called on $this, but another object), it is legal to call a private method. This is done in Silex in several places. However, I would be quite happy if this were public. I think (but don't really know) that people may eventually ask for this method if it's not present.
There was a problem hiding this comment.
tl;dr This code actually works. But maybe we should make it public :)
|
Can you rebase? |
We need this because when you import a RouteCollection and this has a resource on it, we need to pass that resource forward to the RouteCollectionBuilder
549a3e2 to
3b67daa
Compare
|
Rebased! And I just realized I had originally pushed this branch to |
|
👍 |
|
Thank you @weaverryan. |
…tionBuilder (weaverryan) This PR was merged into the 2.8 branch. Discussion ---------- Re-adding the ability to add a resource to the RouteCollectionBuilder | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a At the end of #15778, I removed the ability to add a resource to the RouteCollectionBuilder. But, this is needed (at least internally): if you import, the returned RouteCollection may have resources, which need to be brought forward into the new RouteCollectionBuilder (the code in `import()` to do this already exists, but is calling an undefined `addResource()` method). This adds the test with minimal code to fix. P.S. Fabien told me to remove `addResource` originally... so isn't this *kind of* his fault? Commits ------- 3b67daa Re-adding the ability to add a resource to the RouteCollectionBuilder
At the end of #15778, I removed the ability to add a resource to the RouteCollectionBuilder. But, this is needed (at least internally): if you import, the returned RouteCollection may have resources, which need to be brought forward into the new RouteCollectionBuilder (the code in
import()to do this already exists, but is calling an undefinedaddResource()method).This adds the test with minimal code to fix.
P.S. Fabien told me to remove
addResourceoriginally... so isn't this kind of his fault?