Skip to content

Use bash-cache command for .strings validation #19624

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Use bash-cache command for .strings validation #19624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77248c5
Use `bash-cache` command for `.strings` validation via version 2.12.0
mokagio Nov 18, 2022
45ac4e8
Use same emoji as the plugin for the `.strings` generation lint step
mokagio Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 1 addition & 37 deletions .buildkite/commands/lint-localized-strings-format.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,7 @@
#!/bin/bash -eu

echo "--- :rubygems: Setting up Gems"
install_gems

echo "--- :cocoapods: Setting up Pods"
install_cocoapods

echo "--- :writing_hand: Copy Files"
mkdir -pv ~/.configure/wordpress-ios/secrets
cp -v fastlane/env/project.env-example ~/.configure/wordpress-ios/secrets/project.env
Comment on lines 3 to 5
Copy link
Contributor Author

@mokagio mokagio Nov 18, 2022

Choose a reason for hiding this comment

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

I wonder whether we would benefit from removing the before_all check for this env file and instead rely on the lanes that need secret values to check for their availability.

WordPress-iOS/fastlane/Fastfile

Lines 124 to 126 in adbb17e

unless File.file?(PROJECT_ENV_FILE_PATH)
UI.user_error!("project.env not found at #{PROJECT_ENV_FILE_PATH}: Make sure your configuration is up to date with `rake dependencies`")
end

This is the first time the global check bothers me. I guess that's because it's the first time I need to setup the file only to satisfy the check, not because the code I run needs a value from it.

Copy link
Contributor

@AliSoftware AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

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

I actually wondered about this in the past too.

Probably for another PR, and even a small "project" to not only change this on all repos, but also probably get rid of the requirement for ~/.*-env.default files too (which have been bothering me too, not only because those files clutters my $HOME, but also because I don't use those in practice, as I put all my tokens and the likes that we expect in those files… in my ~/.zshrc anyway (since they are mostly the same values for all repos, and I prefer having a single source of truth than one per project)

Copy link
Contributor Author

@mokagio mokagio Dec 2, 2022

Choose a reason for hiding this comment

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

+1

We could devise a system that checks the environment for all the required tokens, then delegate to each user how to provide them.

Copy link
Contributor

@AliSoftware AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

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

It happens that I also just talked about a similar topic with @rynaardb minutes ago, because putting all my tokens in ~/.zshrc like I've been doing will create an issue now that I'll have to do Tumblr releases, which will likely use similar ENV var names for GitHub and Buildkite tokens and the like… but will need different values for it.

My braindump idea to improve on that while avoid polluting $HOME so that each user can provide the required tokens, but still be able to have different tokens (and different values for the same ENV vars) for different repos would basically be:

local_env_file = File.join(PROJECT_ROOT_FOLDER, 'local.env')
Dotenv.load(local_env_file) if File.exist?(local_env_file)

And then add local.env to the .gitignore (†).

That way users will have the liberty to either not use that local.env file at all and just declare their ENV vars in ~/.zshrc or wherever they prefer… or, if that's more convenient for them (especially for Release Managers who do releases of different apps that require different tokens), create a local.env file at the root of the dir where they cloned the repo and use that.

(†) The name of the local.env file is inspired from the name of the local.properties file used in a similar fashion for Android Projects to define Java properties only for the local user)


echo "--- Lint Localized Strings Format"
LOGS=logs.txt
set +e
set -o pipefail
bundle exec fastlane generate_strings_file_for_glotpress skip_commit:true | tee $LOGS
EXIT_CODE=$?
set -e

echo $EXIT_CODE

if [[ $EXIT_CODE -ne 0 ]]; then
# Strings generation finished with errors, extract the errors in an easy-to-find section
echo "--- Report genstrings errors"
ERRORS=errors.txt
echo "Found errors when trying to run \`genstrings\` to generate the \`.strings\` files from \`*LocalizedStrings\` calls:" | tee $ERRORS
echo '' >> $ERRORS
# Print the errors inline.
#
# Notice the second `sed` call that removes the ANSI escape sequences that
# Fastlane uses to color the output.
grep -e "\[.*\].*genstrings:" $LOGS \
| sed -e 's/\[.*\].*genstrings: error: /- /' \
| sed -e $'s/\x1b\[[0-9;]*m//g' \
| sort \
| uniq \
| tee -a $ERRORS
# Annotate the build with the errors
cat $ERRORS | buildkite-agent annotate --style error --context genstrings
fi

exit $EXIT_CODE
lint_localized_strings_format
Comment on lines -13 to +7
Copy link
Contributor

@AliSoftware AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

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

🧹 🧹 🧹 ✨

7 changes: 2 additions & 5 deletions .buildkite/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
common_params:
# Common plugin settings to use with the `plugins` key.
- &common_plugins
- automattic/bash-cache#2.8.0
- automattic/bash-cache#2.12.0
- automattic/git-s3-cache#v1.1.3:
bucket: "a8c-repo-mirrors"
repo: "wordpress-mobile/wordpress-ios/"
Expand Down Expand Up @@ -123,10 +123,7 @@ steps:
plugins: *common_plugins
agents:
queue: "android"
# Runs the `.strings` generation automation to ensure all the
# `LocalizedString` calls in the code can be correctly parsed by Apple's
# `genstrings`.
- label: "Lint Localized Strings Format"
- label: ":sleuth_or_spy: Lint Localized Strings Format"
command: .buildkite/commands/lint-localized-strings-format.sh
plugins: *common_plugins
env: *common_env