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

chore: update edx-i18n-tools #31174

Merged
merged 1 commit into from
Oct 19, 2022
Merged

Conversation

rgraber
Copy link
Contributor

@rgraber rgraber commented Oct 19, 2022

Description

Update edx-i18n-tools to bring in a fix for the transifex pull command.

Supporting information

The original issue is here: edx/edx-arch-experiments#77 (private to 2U/OCM)
The fix in edx-i18n-tools is here: openedx/i18n-tools#119

Testing instructions

git clean -fdX conf/locale
i18n_tool transifex pull

Copy link
Contributor

@robrap robrap left a comment

Choose a reason for hiding this comment

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

[nit/question] I wonder if "chore" is the right label, or "fix", when pulling in a fix.

@rgraber
Copy link
Contributor Author

rgraber commented Oct 19, 2022

[nit/question] I wonder if "chore" is the right label, or "fix", when pulling in a fix.

I think chore makes the most sense since we'd have to update this eventually anyway as part of general maintenance. We're just doing our chores a bit earlier.

@rgraber rgraber merged commit cc0a07d into master Oct 19, 2022
@rgraber rgraber deleted the rsgraber/20221019-update-i18n-tools branch October 19, 2022 16:15
@robrap
Copy link
Contributor

robrap commented Oct 19, 2022

I think chore makes the most sense since we'd have to update this eventually anyway as part of general maintenance. We're just doing our chores a bit earlier.

Ok. I called it a "nit" because I agree that "chore" is what would have happened if we waited. However - I also just created a PR against the OEP to better explain my thoughts on why I would have used "fix" in this case. See openedx/open-edx-proposals#400 if interested.

@rgraber
Copy link
Contributor Author

rgraber commented Oct 19, 2022

Oof, should have read the OEP better. I didn't realize it was already suggested in there that 'chore' may not be appropriate. I'm actually not 100% sure I agree but I do see the point.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@robrap
Copy link
Contributor

robrap commented Oct 19, 2022

@rgraber: I don't think the OEP does say it is not appropriate. The OEP is a little vague, but I think it is just warning that there may be information hidden behind "chore". Either way, this is pretty low priority for us. Might just be useful to people who need to go through the commit history for release notes.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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