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

Include default msgmerge_options, adding --no-fuzzy-matching for custom makemessages command #12019

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

robdivincenzo
Copy link
Collaborator

@robdivincenzo robdivincenzo commented Mar 7, 2024

Description

This will update the custom makemessages command to include default msgmerge_options as well as the --no-fuzzy-matching. As of 4.1, msgmerge uses the --update flag by default to update .po file: django/django@4bfe8c0

Unless there's a reason not to use them, including default options will help keep it stable going forward. Default options being included are now:

-q, --backup=none, --previous, --update

And for reference, here are the descriptions of the options: https://www.gnu.org/software/gettext/manual/html_node/msgmerge-Invocation.html

Link to sample test page:
Related PRs/issues: #11825

To test

  1. Check out this branch
  2. Run inv makemessages
  3. Verify .po files in local l10n repo are now updating and that output is as expected

@robdivincenzo robdivincenzo changed the title [WIP] Include default msgmerge_options, adding --no-fuzzy-matching Include default msgmerge_options, adding --no-fuzzy-matching for custom makemessages command Mar 7, 2024
@robdivincenzo robdivincenzo marked this pull request as ready for review March 7, 2024 15:41
@robdivincenzo
Copy link
Collaborator Author

Also tagging @TheoChevalier for review since there are a few new default options being included with the custom command.

@robdivincenzo robdivincenzo temporarily deployed to foundation-s-11825-fix--93bpwi March 7, 2024 15:43 Inactive
Copy link
Contributor

@TheoChevalier TheoChevalier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @robdivincenzo for the fix! I’ve tested this PR and can confirm this works

Copy link
Contributor

@jhonatan-lopes jhonatan-lopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 thanks @robdivincenzo

@robdivincenzo robdivincenzo temporarily deployed to foundation-s-11825-fix--93bpwi March 13, 2024 14:32 Inactive
@robdivincenzo robdivincenzo merged commit 4de3f58 into main Mar 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants