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

add ``override`` option to ``add_translation_dirs`` #2902

Merged

Conversation

Projects
None yet
2 participants
@mmerickel
Copy link
Member

commented Jan 17, 2017

I debated with myself for a very long time whether to set override=True by default or not and in the end I simply cannot come up with a compelling reason to break bw-compat. I took a page out of pyramid_jinja2's book with add_jinja2_search_path which also adds new items with lower priority than previously added items. This is consistent with that, as well as previous behaviors.

fixes #1474

@mmerickel mmerickel force-pushed the mmerickel:allow-add-translation-dirs-override branch from bc8db9b to 3d12086 Jan 17, 2017

@mmerickel mmerickel force-pushed the mmerickel:allow-add-translation-dirs-override branch from 3d12086 to d009c0b Jan 17, 2017

@mmerickel mmerickel merged commit a6f903b into Pylons:master Jan 17, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

mmerickel added a commit that referenced this pull request Jan 17, 2017

@piotr-dobrogost

This comment has been minimized.

Copy link

commented Jan 17, 2017

add_jinja2_search_path which also adds new items with lower priority than previously added items

Heh, then it's the wrong default there, too :)

I simply cannot come up with a compelling reason to break bw-compat.

The fact no one before @rbu raised this means that those, who use add_translation_dirs use it only to add translations and not to overwrite them. This means they have no overwrites intended to work because if they had got any they wouldn't have worked and they would have submitted issue. Conclusion; if overwrite=True breaks something it means it was already broken (by having needless overwrites not meant to overwrite).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.