Makes url generation strategy default value configurable#751
Makes url generation strategy default value configurable#751maechler wants to merge 1 commit intoapi-platform:masterfrom maechler:feature/urlGenerationStrategyByConfig
Conversation
maechler
commented
Sep 12, 2016
maechler
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | api-platform/api-platform#128 |
| License | MIT |
src/Api/IriConverterInterface.php
Outdated
| * @return string | ||
| */ | ||
| public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::ABS_PATH) : string; | ||
| public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::CONFIG) : string; |
There was a problem hiding this comment.
Why is it needed to add this new constant? IMO everything should be set in the Configuration and Extension classes.
There was a problem hiding this comment.
Because otherwise the $referenceType parameter would be set to UrlGeneratorInterface::ABS_PATH and we would have no idea whether we should use the default config value or whether someone wanted to overwrite the default value on the method call.
Do you have another idea?
There was a problem hiding this comment.
Why cannot we let the current default value? The default value will be injected everywhere by the DIC anyway, but it will let users using components directly (without Symfony) to have sensible defaults.
There was a problem hiding this comment.
I agree that this makes no sense... 😛
|
I have started to move the default reference type a bit up. There are really a lot of changes to be made and I have only injected the default reference type to half of the classes of which it would be necessary to. |
|
Indeed you can do it locally using a decorator, but IMO it would be a very nice improvement to be able to configure that globally in core. |
|
@dunglas Would you be ok with the way I injected the configuration in all classes that need it? If you have better ideas to implement this let me know. |
|
@maechler sorry for being so long to reply. I need to review this more deeply. I think it is possible to pass the constant to use in the function parameter in most case instead of adding global default values as states. |
|
@dunglas Never mind! That would be fine too of course. Anyways..you guys are doing a great job with the development of api-platform! Thanks! |
|
@antograssiot No worries! It is good to see that this issue is being revived, thanks. |
5 participants
