-
Notifications
You must be signed in to change notification settings - Fork 536
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
Make locales extraction script work with and without token #14049
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,10 @@ | |
# - Update your code | ||
# - Extract new strings and push to the .po files | ||
# | ||
# If you're running the script manually please make sure to expose | ||
# For the fully automated experience make sure to expose | ||
# the following variables to the environment: | ||
# - GITHUB_TOKEN (to the github token of addons-robot, | ||
# talk to tofumatt or cgrebs) | ||
# talk to @diox or @muffinresearch) | ||
# - TRAVIS_REPO_SLUG="mozilla/addons-server" | ||
# - TRAVIS_BRANCH="master" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't appear to be used at all (despite the erroring if it's not set). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is purely if used in travis to make sure it's not running against branches other than master There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay, that makes sense. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmm in the case of travis, |
||
|
@@ -72,9 +72,11 @@ function extract_locales { | |
echo "done." | ||
} | ||
|
||
|
||
function commit_and_push { | ||
function git_commit { | ||
git commit -m "$MESSAGE" --author "$ROBOT_NAME <$ROBOT_EMAIL>" --no-gpg-sign locale/*/LC_MESSAGES/*.po locale/templates/ | ||
} | ||
|
||
function git_push { | ||
git push -q "https://addons-robot:[email protected]/$TRAVIS_REPO_SLUG/" | ||
} | ||
|
||
|
@@ -98,21 +100,23 @@ function create_auto_pull_request { | |
echo "auto merge pull request is created ..." | ||
} | ||
|
||
if [ "$TRAVIS_BRANCH" != "master" ]; then | ||
echo "This commit was made against the $TRAVIS_BRANCH and not the master! No extract!" | ||
exit 0 | ||
fi | ||
|
||
if [ "GITHUB_TOKEN" == "" ] | ||
then | ||
echo "Must provide github token" | ||
exit 0 | ||
fi | ||
|
||
init_environment | ||
|
||
extract_locales | ||
|
||
commit_and_push | ||
git_commit | ||
|
||
if [ "GITHUB_TOKEN" = "" ] | ||
then | ||
echo "No Github token provided, you'll have to push the branch and create the pull request yourself." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand why we can't create the pull request without the github API access but we could still push the branch like https://github.com/mozilla/addons-frontend/pull/9375/files#diff-cae1c8c05fe95719db14844a8f236ab1R76 |
||
else | ||
if [ "$TRAVIS_BRANCH" != "master" ]; then | ||
echo "This commit was made against the $TRAVIS_BRANCH and not the master! No extract!" | ||
exit 0 | ||
fi | ||
git_push | ||
|
||
create_auto_pull_request | ||
fi | ||
|
||
|
||
create_auto_pull_request |
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.
Can't we provide this as a default in the file?
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.
As
TRAVIS_BRANCH
this is automatically present in travisThere 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.
I'm thinking for local use so we don't have to define extra variables to make it run.
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.
Yeah I'll make sure no variables are needed for manual usage