-
Notifications
You must be signed in to change notification settings - Fork 37
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: release docs and script updates | FC-12 #6056
Conversation
Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
c6955c1
to
8dce095
Compare
if not slug: | ||
raise RuntimeError( | ||
'Error: Cannot determine Transifex project slug. Set `TRANSIFEX_PROJECT_SLUG` environment variable to ' | ||
'"openedx-translations" or "openedx-translations-redwood".' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to mention redwood
specifically here, or should this error be more generic (something like openedx-translations-{release-name}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I think we should use release-name
everywhere in these docs, because otherwise we'll definitely forget to update this for every release.
You could provide a link to https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4191191044/Open+edX+Releases+Homepage for the name of the current and next releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good note, done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of requested changes but looks great, thank you
README.rst
Outdated
Tools for repository maintainers | ||
******************************** | ||
|
||
This repository contains both GitHub Actions workflows and Makefile programs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the GHA workflows defined in https://github.com/openedx/.github ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflows are specific to this repo. For example: extract-translation-source-files.yml
pulls from a bunch of repos throughout the org and makes commits to this repo adding source strings. No github actions are needed on other repos for this process to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good note. I've added a link to avoid confusion.
This repository makes an unusual use of GitHub Actions, therefore it's a special noteworthy case.
if not slug: | ||
raise RuntimeError( | ||
'Error: Cannot determine Transifex project slug. Set `TRANSIFEX_PROJECT_SLUG` environment variable to ' | ||
'"openedx-translations" or "openedx-translations-redwood".' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I think we should use release-name
everywhere in these docs, because otherwise we'll definitely forget to update this for every release.
You could provide a link to https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4191191044/Open+edX+Releases+Homepage for the name of the current and next releases.
bae56cb
to
42fa896
Compare
@sarina I've addressed the notes, please let me know if anything else is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @OmarIthawi sorry I made a comment too fast, and didn't indicate I wanted that change throughout the document.
0d88859
to
ceef6b6
Compare
12c736c
to
5d8ed22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Suggest removing a couple lines that I think are holdovers otherwise good to merge
d305a06
to
22bf377
Compare
Great! Thanks @brian-smith-tcril and @sarina!! |
@OmarIthawi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
This refreshes the repository docs and scripts with the Redwood release.
Done
sync_translations
script and its workflow. Its job has been doneTRANSIFEX_PROJECT_SLUG
inmake fix_transifex_resource_names_dry_run
Not done