[HttpKernel] Parse bundle name+namespace at once #20117
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
ro0NL
commented
Oct 1, 2016
•
ro0NL
commented
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |
|
Hey, Since this is not a bug fix. Shouldn't this be applied against |
|
That's definitely for master |
lemoinem
suggested changes
Oct 2, 2016
|
|
||
| private function parseClassName() | ||
| { | ||
| $class = get_class($this); |
There was a problem hiding this comment.
Since this is intended to target master, I'm guessing you could replace that by static::class?
Not sure.. it must gain something i guess :) I thought it was nice little tweak for 2.7 as well. Ie. imo its not a bug fix nor a new feature (new feature for the user that is). I guess the thing is we should be safe with refactoring.. as things can easily go wrong 😅 Ill target master 👍 |
|
Thank you @ro0NL. |
fabpot
added a commit
that referenced
this pull request
Oct 2, 2016
This PR was merged into the 3.2-dev branch. Discussion ---------- [HttpKernel] Parse bundle name+namespace at once | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | comma-separated list of tickets fixed by the PR, if any | License | MIT | Doc PR | reference to the documentation PR, if any Commits ------- 81b2922 parse bundle name+namespace at once
fabpot
added a commit
that referenced
this pull request
Dec 17, 2016
This PR was merged into the 3.3-dev branch. Discussion ---------- [HttpKernel] Fix Bundle name regression | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20117 | License | MIT | Doc PR | N/A The bundle name can be set manually instead of being guessed from the class name, as the property is protected. However, a regression prevents this name to be used, as calling `Bundle::getNamespace()` recomputes the bundle name from class instead. The ability to name explicitly bundles is appreciable when dealing with "virtual" ones, or when providing bundles in a library under a `Vendor\MyPackage\Bridge\Symfony\Bundle` namespace. No need to rename the bundle class `VendorMyPackageBundle` which will make the instantiation in `Kernel::registerBundle()` quite ugly: ```diff - new Vendor\MyPackage\Bridge\Symfony\Bundle\VendorMyPackageBundle() + new Vendor\MyPackage\Bridge\Symfony\Bundle\Bundle() ``` What about removing `Bundle::parseClassName()` and processing the namespace and bundle name separately, but keeping the `namespace` property introduced in #20117? Commits ------- 3b5127d [HttpKernel] Fix Bundle name regression
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.