Skip to content

Conversation

@lw
Copy link
Member

@lw lw commented Oct 19, 2018

A directory was missing.

We could honestly just use **.py to extract from all Python files in the repo rather than listing all directories individually. I don't think we would have that many false positives (possibly none at all). I think I originally listed the directories because that was what we were doing before adopting Babel and I wanted to have the files processed in the same order to reduce the diff to the cms.pot file, which however ended being quite large regardless.


This change is Reviewable

@codecov
Copy link

codecov bot commented Oct 19, 2018

Codecov Report

Merging #1047 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1047      +/-   ##
==========================================
- Coverage   61.98%   61.96%   -0.03%     
==========================================
  Files         227      227              
  Lines       16405    16405              
==========================================
- Hits        10169    10165       -4     
- Misses       6236     6240       +4
Flag Coverage Δ
#functionaltests 46.08% <ø> (-0.06%) ⬇️
#unittests 43.13% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
cms/io/PsycoGevent.py 65.71% <0%> (-2.86%) ⬇️
cms/io/priorityqueue.py 92.7% <0%> (-2.19%) ⬇️
cms/server/admin/handlers/task.py 39.84% <0%> (-1.18%) ⬇️
cms/db/base.py 82.97% <0%> (-1.07%) ⬇️
cms/service/ProxyService.py 57.44% <0%> (-1.07%) ⬇️
cms/server/admin/handlers/dataset.py 24.76% <0%> (-0.93%) ⬇️
cms/service/workerpool.py 62.22% <0%> (-0.56%) ⬇️
cms/grading/Sandbox.py 67.87% <0%> (-0.37%) ⬇️
cms/io/rpc.py 91.46% <0%> (-0.35%) ⬇️
cms/service/esoperations.py 78.72% <0%> (ø) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7101adb...c2b1110. Read the comment docs.

Copy link
Member

@stefano-maggiolo stefano-maggiolo left a comment

Choose a reason for hiding this comment

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

I think the reason we didn't want **py is because of the directory created by setup.py?

Also: is it possible that when I regenerated the po I also deleted some of the existing translation due to this bug? Can you check and if so restore the state of before?

Reviewable status: 0 of 2 files reviewed, all discussions resolved

Copy link
Member

@stefano-maggiolo stefano-maggiolo left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member Author

@lw lw left a comment

Choose a reason for hiding this comment

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

Ok, I will leave the explicit listing of the directories.

I didn't understand what you're referring to: you updated the .pot file, which I regenerated in this PR, but you didn't touch the .po files. (In fact they haven't been updated in at least 6 month, whereas the pot with the missing strings was generated just one month ago). So I don't think any translation was lost due to this issue. Moreover, as we saw in #1045, translations for messages that disappeared from the pot are just commented out, not removed.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@lw lw merged commit 23eb239 into cms-dev:master Oct 20, 2018
@lw lw deleted the fix_babel_extract branch October 20, 2018 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants