Use bash-cache command for `.strings` validation #19624

mokagio · 2022-11-18T13:28:18Z

The work from #19553 is now part of bash-cache.

Testing

CI is green in this step, which validates the happy path. I recycled the same test PR as #19553 by force pushing and you can see it still fails

Regression Notes

Potential unintended areas of impact – N.A.
What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
What automated tests I added (or what prevented me from doing so) – N.A.

I have completed the Regression Notes.
I have considered adding unit tests for my changes. N.A.
I have considered adding accessibility improvements for my changes. N.A.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

mokagio · 2022-11-18T13:33:15Z

.buildkite/commands/lint-localized-strings-format.sh

 echo "--- :writing_hand: Copy Files"
 mkdir -pv ~/.configure/wordpress-ios/secrets
 cp -v fastlane/env/project.env-example ~/.configure/wordpress-ios/secrets/project.env


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.

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)

+1

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

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)

wpmobilebot · 2022-11-18T13:59:22Z

You can test the changes in WordPress from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr19624-45ac4e8 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

wpmobilebot · 2022-11-18T13:59:58Z

You can test the changes in Jetpack from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr19624-45ac4e8 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

mokagio · 2022-11-18T15:27:27Z

.buildkite/pipeline.yml

  # Common plugin settings to use with the `plugins` key.
  - &common_plugins
-    - automattic/bash-cache#2.8.0
+    - automattic/bash-cache#mokagio/genstring-in-ci


Will need to push a new commit or rewrite this one to point to the next bash-cache version once Automattic/a8c-ci-toolkit-buildkite-plugin#36 lands.

peril-wordpress-mobile · 2022-12-02T06:07:53Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

AliSoftware

👌

AliSoftware · 2022-12-02T10:24:48Z

.buildkite/commands/lint-localized-strings-format.sh

 echo "--- :writing_hand: Copy Files"
 mkdir -pv ~/.configure/wordpress-ios/secrets
 cp -v fastlane/env/project.env-example ~/.configure/wordpress-ios/secrets/project.env


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)

AliSoftware · 2022-12-02T10:27:47Z

.buildkite/commands/lint-localized-strings-format.sh

-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


🧹 🧹 🧹 ✨

mokagio commented Nov 18, 2022

View reviewed changes

mokagio mentioned this pull request Nov 18, 2022

Add command to run .strings file generation Automattic/a8c-ci-toolkit-buildkite-plugin#36

Merged

mokagio commented Nov 18, 2022

View reviewed changes

mokagio force-pushed the mokagio/genstrings-plugin branch 3 times, most recently from 35306ef to 2176ece Compare November 20, 2022 11:45

mokagio mentioned this pull request Nov 23, 2022

Use a unique key for safariButtonAccessibilityLabel #19651

Merged

4 tasks

mokagio added 2 commits November 25, 2022 07:07

Use bash-cache command for .strings validation via version 2.12.0

77248c5

Use same emoji as the plugin for the .strings generation lint step

45ac4e8

mokagio force-pushed the mokagio/genstrings-plugin branch from 2176ece to 45ac4e8 Compare November 25, 2022 06:07

mokagio marked this pull request as ready for review December 2, 2022 06:07

mokagio self-assigned this Dec 2, 2022

mokagio requested a review from a team December 2, 2022 06:07

AliSoftware approved these changes Dec 2, 2022

View reviewed changes

mokagio merged commit bff014b into trunk Dec 2, 2022

mokagio deleted the mokagio/genstrings-plugin branch December 2, 2022 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use bash-cache command for `.strings` validation #19624

Use bash-cache command for `.strings` validation #19624

Uh oh!

mokagio commented Nov 18, 2022 •

edited

Loading

Uh oh!

mokagio Nov 18, 2022

Uh oh!

AliSoftware Dec 2, 2022

Uh oh!

mokagio Dec 2, 2022

Uh oh!

AliSoftware Dec 2, 2022

Uh oh!

wpmobilebot commented Nov 18, 2022 •

edited

Loading

Uh oh!

wpmobilebot commented Nov 18, 2022 •

edited

Loading

Uh oh!

mokagio Nov 18, 2022

Uh oh!

peril-wordpress-mobile bot commented Dec 2, 2022

Uh oh!

AliSoftware left a comment

Uh oh!

AliSoftware Dec 2, 2022

Uh oh!

AliSoftware Dec 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	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

Use bash-cache command for .strings validation #19624

Use bash-cache command for .strings validation #19624

Uh oh!

Conversation

mokagio commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Regression Notes

Uh oh!

mokagio Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mokagio Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented Dec 2, 2022

Uh oh!

AliSoftware left a comment

Choose a reason for hiding this comment

Uh oh!

AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

AliSoftware Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Use bash-cache command for `.strings` validation #19624

Use bash-cache command for `.strings` validation #19624

mokagio commented Nov 18, 2022 •

edited

Loading

wpmobilebot commented Nov 18, 2022 •

edited

Loading

wpmobilebot commented Nov 18, 2022 •

edited

Loading