Skip to content
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

[DX] Add sylius prefixes to normalizers ALREADY_CALLED consts values to avoid name collision #12970

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

lchrusciel
Copy link
Member

@lchrusciel lchrusciel commented Aug 17, 2021

Q A
Branch? master
Bug fix? kind of
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

While adding custom normalizer, it may be tempting to use the same name for the flag. This may lead to skipping part of normalization. It would be better to prefix our config keys with sylius and make official recommendation, that such keys in end projects should be prefixed with app

@lchrusciel lchrusciel added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Aug 17, 2021
@lchrusciel lchrusciel requested a review from a team as a code owner August 17, 2021 15:23
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Aug 17, 2021
@GSadee GSadee merged commit 2c8755e into Sylius:master Aug 18, 2021
@GSadee
Copy link
Member

GSadee commented Aug 18, 2021

Thanks, Łukasz! 🎉

@lchrusciel lchrusciel deleted the prefixed-keys branch August 18, 2021 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants