Try to auto-detect a bundle's model namespace by default #9040
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
lchrusciel
approved these changes
Dec 19, 2017
|
I'm wondering if it shouldn't go to 1.0 branch. /cc @pamil |
|
@lchrusciel I'll rather keep 1.0 branch only for bugfixes, this is a nice DX improvement so the next minor is the best choice. |
pamil
suggested changes
Dec 27, 2017
| @@ -97,9 +97,9 @@ protected function getDoctrineMappingDirectory(): string | |||
| * | |||
| * @return string | |||
| */ | |||
| protected function getModelNamespace(): ?string | |||
| protected function getModelNamespace(): string | |||
There was a problem hiding this comment.
Changing the return typehint by removing nullability is a no-go zone :) It would break 3rd party applications if they're using the currently correct method signature with ?string return typehint.
|
@mbabker thanks for this PR, making plugins development easier is vital for Sylius 🎉 Let's change that typehint and it's good to go. |
|
Updated, though you should really add that change to the 2.0 backlog 😉 |
pamil
approved these changes
Dec 27, 2017
|
Thank you Michael! 🥇 |
wadjeroudi
added a commit
to printoclock/Sylius
that referenced
this pull request
Jan 9, 2018
v1.0.7 - [Sylius#9075](Sylius#9075) Test sylius:install command on Travis (@pamil) - [Sylius#9070](Sylius#9070) Extend Travis build matrix & setup extra jobs running nightly (1.1) (@pamil) - [Sylius#9071](Sylius#9071) Update composer.json (1.0) (@pamil) - [Sylius#9069](Sylius#9069) Extend Travis build matrix & setup extra jobs running nightly (1.0) (@pamil) - [Sylius#9065](Sylius#9065) Various Travis changes (@pamil) - [Sylius#9048](Sylius#9048) Exclude the web/media directory from firewall restrict list. (@Hailong) - [Sylius#9040](Sylius#9040) Try to auto-detect a bundle's model namespace by default (@mbabker) - [Sylius#9063](Sylius#9063) Require Symfony ^3.4 in components & bundles (@pamil) - [Sylius#9055](Sylius#9055) Remove PHP <= 7.1 Polyfills (@stefandoorn) - [Sylius#9052](Sylius#9052) Fixes history issue when adding to cart (@gabriel-cardoso) - [Sylius#9061](Sylius#9061) Require Symfony ^3.4 (@pamil) - [Sylius#9060](Sylius#9060) Remove the need for using sudo on Travis (@pamil) - [Sylius#9059](Sylius#9059) Make Travis use the most recent Chromedriver (@pamil)
pamil
added a commit
to pamil/Sylius
that referenced
this pull request
May 7, 2019
Try to auto-detect a bundle's model namespace by default
pamil
added a commit
to pamil/Sylius
that referenced
this pull request
May 7, 2019
Try to auto-detect a bundle's model namespace by default
pamil
added a commit
to pamil/Sylius
that referenced
this pull request
May 7, 2019
Try to auto-detect a bundle's model namespace by default
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.
The
AbstractResourceBundleright now requires that bundles/plugins define a model namespace. Following conventions in core, by default this is presumably<BundleNamespace>\Model. So instead of forcing child classes to define this we can make an assumption by default as a small DX improvement.For the most part this doesn't impact core as most bundles are using component models (except for the two changed here). But this can impact the plugin ecosystem and local app development by requiring one less method to set up.