feat: restore PO-Revision-Date but with fixed value #140

shadinaif · 2023-09-27T12:56:48Z

External packages are forcing validation on PO-Revision-Date. Therefore, we are restoring both POT-Creation-Date and PO-Revision-Date but setting a fixed value for them to avoid having a lot of false (git diff) lines. Note: (2023-06-13 is Palm release date)

This is a fix for a defect reported in credentials repository: #131

Reverts part of the work done here #129

This contribution is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

openedx-webhooks · 2023-09-27T12:56:52Z

Thanks for the pull request, @shadinaif! 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:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

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.

codecov · 2023-09-27T13:00:36Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (ac49869) 99.77% compared to head (f1ba793) 99.78%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   99.77%   99.78%   +0.01%     
==========================================
  Files          10       11       +1     
  Lines         439      461      +22     
  Branches       31       33       +2     
==========================================
+ Hits          438      460      +22     
  Partials        1        1

Files	Coverage Δ
tests/test_extract.py	`100.00% <100.00%> (ø)`
tests/test_integration.py	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shadinaif · 2023-09-27T13:24:52Z

Ready for review @OmarIthawi @brian-smith-tcril

i18n/extract.py

OmarIthawi · 2023-09-28T07:38:08Z

Thanks @shadinaif!! Some tests are failing. Would you mind taking a look?

Also I think it makes sense to have an integration test case which runs the following two commands on a sample repo test folder:

cd tests/data/mock-django-app
# Needs some python files to generate languages for
i18n_tool extract
i18n_tool validate

I think this will ensure that this error will not happen again, what do you think?

i18n/extract.py

OmarIthawi

Thanks @shadinaif. Few more minor notes and it should be ready.

tests/test_integration.py

OmarIthawi

Thanks a lot @shadinaif! I've tested it with the following:

$ git clone [email protected]:Zeit-Labs/i18n-tools.git --branch=shadinaif/restore-revision-date
$ cd credentials
$ make requirements  # credentials requirements
$ pip install -e ../i18n-tools
$ make extract_translations

Output seems to be working okay!

$ make validate_translations
.....
INFO:i18n.validate:No problems found in /home/omar/work/openedx/credentials/credentials/conf/locale/es_AR/LC_MESSAGES/django.po
INFO:i18n.execute:msgfmt -c -o /dev/null es_AR/LC_MESSAGES/djangojs.po
INFO:i18n.validate:No problems found in /home/omar/work/openedx/credentials/credentials/conf/locale/es_AR/LC_MESSAGES/djangojs.po
omar@antilop:credentials $ 
...

i18n/extract.py

External packages are forcing validation on PO-Revision-Date. Therefore, we are restoring both POT-Creation-Date and PO-Revision-Date but setting a fixed value for them to avoid having a lot of false (git diff) lines. Note: (2023-06-13 is Palm release date)

awais786 · 2023-10-04T18:58:24Z

@OmarIthawi any thing left in this PR ? waiting for new release to unblock credentials.

OmarIthawi · 2023-10-05T09:26:00Z

Thanks for checking @awais786. We need someone to test it on the credentials repo, I couldn't get to this in a timely manner.

Justin was going to test it, but please feel free to do so.

If it's good, I can merge and cut a new release.

awais786 · 2023-10-05T11:06:13Z

@OmarIthawi openedx/credentials#2221 plz check this. Seems trans are working,

awais786 · 2023-10-05T11:11:58Z

now credentials failing with black issue. I have fixed black path in credential so all set good to go.

black --check .

would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/branch_cleanup.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/__init__.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/changed.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/converter.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/config.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/execute.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/dummy.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/extract.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/generate.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/main.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/segment.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/transifex.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/scripts/podup.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/setup.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/i18n/validate.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/__init__.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_config.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_changed.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_converter.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_dummy.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_extract.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_generate.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_integration.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_transifex.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_segment.py
would reformat /home/runner/work/credentials/credentials/src/edx-i18n-tools/tests/test_validate.py

justinhynes · 2023-10-05T12:17:19Z

I wasn't able to get to this yesterday. I tried it out this morning and I was able to validate translations locally without any further issues. Thanks for the work on this!

awais786 · 2023-10-05T16:32:02Z

@OmarIthawi plz release new version.

justinhynes · 2023-10-05T19:13:52Z

@awais786 That's interesting... Black shouldn't be trying to reformat the files of an external library. 🤔 Worst case maybe we just need to update pyproject.toml to ensure it's properly excluding directories? Or maybe it's a symptom of how the library was installed?

Either way, that problem is definitely on the Credentials side and I'm sure we can figure out a way to work around it.

awais786 · 2023-10-05T19:20:06Z

@awais786 That's interesting... Black shouldn't be trying to reformat the files of an external library. 🤔 Worst case maybe we just need to update pyproject.toml to ensure it's properly excluding directories? Or maybe it's a symptom of how the library was installed?

Either way, that problem is definitely on the Credentials side and I'm sure we can figure out a way to work around it.

I have already fixed openedx/credentials#2221

openedx-webhooks · 2023-10-06T06:09:34Z

@shadinaif 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

omar-nelc · 2023-10-06T07:34:43Z

@awais786 thanks for the review and the release cut. I was about to merge and cut the release, but you've beat me to it.

Thanks again for the thorough report! It's making this tool much better :)

awais786 · 2023-10-06T09:29:43Z

@OmarIthawi Thanks for the quick fix.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 27, 2023

shadinaif force-pushed the shadinaif/restore-revision-date branch 2 times, most recently from 1fce5ae to 2985a2c Compare September 27, 2023 13:00

shadinaif marked this pull request as ready for review September 27, 2023 13:24

shadinaif mentioned this pull request Sep 27, 2023

Defect: PO-Revision-Date being removed causes validation errors in 1.x release #131

Closed

brian-smith-tcril requested a review from justinhynes September 27, 2023 14:02

OmarIthawi reviewed Sep 27, 2023

View reviewed changes

i18n/extract.py Outdated Show resolved Hide resolved

shadinaif force-pushed the shadinaif/restore-revision-date branch 4 times, most recently from 32c0738 to 3e37cb3 Compare September 28, 2023 05:05

shadinaif requested a review from OmarIthawi September 28, 2023 05:05

shadinaif force-pushed the shadinaif/restore-revision-date branch 2 times, most recently from 3eb3afb to 0acac84 Compare September 30, 2023 15:07

OmarIthawi reviewed Sep 30, 2023

View reviewed changes

i18n/extract.py Outdated Show resolved Hide resolved

shadinaif commented Sep 30, 2023

View reviewed changes

i18n/extract.py Show resolved Hide resolved

shadinaif requested a review from OmarIthawi September 30, 2023 15:10

OmarIthawi reviewed Sep 30, 2023

View reviewed changes

tests/test_integration.py Outdated Show resolved Hide resolved

tests/test_integration.py Outdated Show resolved Hide resolved

OmarIthawi self-requested a review September 30, 2023 15:12

shadinaif force-pushed the shadinaif/restore-revision-date branch from 0acac84 to 36d6fda Compare September 30, 2023 15:16

OmarIthawi approved these changes Sep 30, 2023

View reviewed changes

i18n/extract.py Outdated Show resolved Hide resolved

shadinaif force-pushed the shadinaif/restore-revision-date branch from 36d6fda to f1ba793 Compare September 30, 2023 15:32

OmarIthawi requested a review from awais786 October 5, 2023 09:25

awais786 approved these changes Oct 5, 2023

View reviewed changes

awais786 self-requested a review October 5, 2023 11:35

awais786 approved these changes Oct 5, 2023

View reviewed changes

justinhynes approved these changes Oct 5, 2023

View reviewed changes

awais786 merged commit a2cccea into openedx:master Oct 6, 2023
6 checks passed

shadinaif deleted the shadinaif/restore-revision-date branch October 8, 2023 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: restore PO-Revision-Date but with fixed value #140

feat: restore PO-Revision-Date but with fixed value #140

shadinaif commented Sep 27, 2023 •

edited by OmarIthawi

Loading

openedx-webhooks commented Sep 27, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading

shadinaif commented Sep 27, 2023

OmarIthawi commented Sep 28, 2023

OmarIthawi left a comment

OmarIthawi left a comment •

edited

Loading

awais786 commented Oct 4, 2023

OmarIthawi commented Oct 5, 2023

awais786 commented Oct 5, 2023

awais786 commented Oct 5, 2023 •

edited

Loading

justinhynes commented Oct 5, 2023

awais786 commented Oct 5, 2023

justinhynes commented Oct 5, 2023

awais786 commented Oct 5, 2023

openedx-webhooks commented Oct 6, 2023

omar-nelc commented Oct 6, 2023

awais786 commented Oct 6, 2023

feat: restore PO-Revision-Date but with fixed value #140

feat: restore PO-Revision-Date but with fixed value #140

Conversation

shadinaif commented Sep 27, 2023 • edited by OmarIthawi Loading

openedx-webhooks commented Sep 27, 2023 • edited Loading

codecov bot commented Sep 27, 2023 • edited Loading

Codecov Report

shadinaif commented Sep 27, 2023

OmarIthawi commented Sep 28, 2023

OmarIthawi left a comment

Choose a reason for hiding this comment

OmarIthawi left a comment • edited Loading

Choose a reason for hiding this comment

awais786 commented Oct 4, 2023

OmarIthawi commented Oct 5, 2023

awais786 commented Oct 5, 2023

awais786 commented Oct 5, 2023 • edited Loading

justinhynes commented Oct 5, 2023

awais786 commented Oct 5, 2023

justinhynes commented Oct 5, 2023

awais786 commented Oct 5, 2023

openedx-webhooks commented Oct 6, 2023

omar-nelc commented Oct 6, 2023

awais786 commented Oct 6, 2023

shadinaif commented Sep 27, 2023 •

edited by OmarIthawi

Loading

openedx-webhooks commented Sep 27, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading

OmarIthawi left a comment •

edited

Loading

awais786 commented Oct 5, 2023 •

edited

Loading