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

Travis CI: push translation updates (if any) #487

Merged
merged 8 commits into from
Feb 27, 2018
Merged

Travis CI: push translation updates (if any) #487

merged 8 commits into from
Feb 27, 2018

Conversation

plata
Copy link
Collaborator

@plata plata commented Feb 25, 2018

fixes #480

@plata
Copy link
Collaborator Author

plata commented Feb 25, 2018

I couldn't really test this because the script will only run on master.

Copy link
Collaborator

@madoar madoar left a comment

Choose a reason for hiding this comment

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

Can you please add some more comments to the scripts?
Especially to the update_translations.py file.

@plata
Copy link
Collaborator Author

plata commented Feb 25, 2018

I would prefer not to add comments to the old part of update_translations.py because it's not related to this PR. If you tell me where I should add comments in the new code, I will do so.

.travis.yml Outdated
@@ -3,3 +3,9 @@ before_install:
- sudo apt-get install -y python-jsonschema python-pil

script: python ./validate.py

after_success: if [ "$TRAVIS_BRANCH" == "master" ] && [ "$TRAVIS_PULL_REQUEST" == "false" ]; then ./i18n/push_translations.sh; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if it's currently quite clear what the both conditions check, but I think we should add a comment here

for file_name in fnmatch.filter(file_names, '*.po'):
languages.append(os.path.splitext(file_name)[0])
for language in languages:
ps = subprocess.Popen('msgmerge -U i18n/{}.po i18n/keys.pot'.format(language), shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we need a comment here shortly explaining what the commands msgmerge and msgcat do in these cases

for root, dir_names, file_names in os.walk(cwd + '/i18n'):
for file_name in fnmatch.filter(file_names, '*.po'):
languages.append(os.path.splitext(file_name)[0])
for language in languages:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should add a short comment explaining how you find the languages

@plata plata merged commit c3f0db9 into PhoenicisOrg:master Feb 27, 2018
@plata plata deleted the push-translations branch February 27, 2018 17:37
@plata
Copy link
Collaborator Author

plata commented Feb 27, 2018

Mh. That didn't work as expected. For some reason, I can't even see the log in Travis.

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.

update translation files automatically
2 participants