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

[CI] New Github Action to add screenshot diff report #9

Open
wants to merge 57 commits into
base: dev
Choose a base branch
from

Conversation

kdmukai
Copy link
Collaborator

@kdmukai kdmukai commented Dec 30, 2024

Adds a new Github Action to automatically generate a screenshots diff report.

GOAL: To enable Transifex "Reviewers" (essentially admins for their language) to submit their own updated translation PRs and get immediate, automated feedback to see how the new translations affect various screens. This way the translators can iterate extremely quickly on improvements without a developer being a bottleneck.

If they don't like how an updated screen turns out, they can tweak the translation in Transifex, download the new messages.po from Transifex, update their PR, and immediately see the next result. Iterate, iterate, iterate. With no human dependencies.

Current draft instructions for how Transifex Reviewers can create translation PRs (assumes they are not super technical nor experienced with Github): https://gist.github.com/kdmukai/89071afb68a6a9018524c593141a0adb

If this PR is merged, those instructions will be updated at the end for how to review the screenshot changes detected by the CI run.


see example CI runs in my fork: https://github.com/kdmukai/seedsigner-translations/actions

(CI also runs right here in this PR, but there are no translation changes vs dev right now)

Sample resulting CI artifacts: ci-artifacts.zip


The included minified css is copied directly from https://github.com/picocss/pico


I think this should eventually be moved to the main repo, but that is a "high stakes" repo and there is a lot of new mechanism here that contributors will need time to get fully comfortable with. But that being said, this is a pretty crucial tool to have during this initial translations phase. And since this is a lower stakes repo, the bar is somewhat lower for enabling this automation in here first.

@kdmukai
Copy link
Collaborator Author

kdmukai commented Dec 30, 2024

@dbast you are our Github Actions guru and would love your eyes and feedback on this!

@fedebuyito
Copy link

Hi @kdmukai , think I am doing the commit for testing this on wrong repo, right?

image

@fedebuyito
Copy link

Maybe now?

image

@dbast
Copy link

dbast commented Dec 31, 2024

The PR looks pretty good: The action versions are outdated and could be updated actions/upload-artifact@v3 -> actions/upload-artifact@v4, actions/setup-python@v4 -> actions/setup-python@v5, actions/upload-artifact@v3 -> actions/upload-artifact@v3. This also resolves the deprecation warnings shown in https://github.com/SeedSigner/seedsigner-translations/actions/runs/12551708293?pr=9

@kdmukai
Copy link
Collaborator Author

kdmukai commented Dec 31, 2024

@dbast I applied the changes:

  • actions/upload-artifact: v3 -> v4
  • actions/setup-python: v4 -> v5

CI ran successfully with those changes: https://github.com/SeedSigner/seedsigner-translations/actions/runs/12562703726

No deprecation warnings in the output (just the note that ubuntu-latest is versioning up soon).

@jdlcdl
Copy link

jdlcdl commented Dec 31, 2024

Concept ACK

I cannot wait to see this in action, it will make a huge difference to shorten the feedback loop for translators.

I think I saw an artifacts file from a fedebuyito commit, changing "borrar to supr"? but I'm unsure about when that run occured and/or if it reflects latest changes. Will watch this pr with hope.

Have a safe end-of-year celebration fellows! All the best for 2025.

@dbast
Copy link

dbast commented Dec 31, 2024

Sorry, forgot this in my previous comment: actions/checkout@v3 can also be updated to actions/checkout@v4.

Thanks and all the best for 2025!

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.

4 participants