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

I18n Workflow Fix #642

Merged
merged 11 commits into from
Oct 28, 2024
Merged

Conversation

zyf722
Copy link
Contributor

@zyf722 zyf722 commented Oct 28, 2024

What type of PR is this? (check all applicable)

  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • ♻️ Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert
  • 🌐 Internationalization / Translation

Description

This PR is intended to fix issues with the i18n workflow introduced in the previous i18n PR.

A helper script i18n-lokalise-json is also added to help converting translation from .ts to Lokalise .json files.

Breaking changes

  • Use <owner>/<branch> for PR_BRANCH in CI to avoid potential naming collisions @ 4dc6dbd

Related Tickets & Documents

Copy link

netlify bot commented Oct 28, 2024

βœ… Deploy Preview for livecodes ready!

Name Link
πŸ”¨ Latest commit 6aad770
πŸ” Latest deploy log https://app.netlify.com/sites/livecodes/deploys/671fadcbb7b3340008ff745a
😎 Deploy Preview https://deploy-preview-642--livecodes.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@zyf722 zyf722 mentioned this pull request Oct 28, 2024
Copy link

@hatemhosny hatemhosny merged commit e08c415 into live-codes:develop Oct 28, 2024
11 checks passed
@hatemhosny
Copy link
Collaborator

Hi @zyf722

So the i18n-update-push, i18n-update-pull & creating the PR worked.

However, the created PR did not trigger the CI build & tests. See https://github.com/orgs/community/discussions/33804

You may want to use the secret GH_ACTIONS_REPO_PAT instead of GITHUB_TOKEN

@zyf722
Copy link
Contributor Author

zyf722 commented Oct 30, 2024

However, the created PR did not trigger the CI build & tests. See https://github.com/orgs/community/discussions/33804

You may want to use the secret GH_ACTIONS_REPO_PAT instead of GITHUB_TOKEN

Thank you for pointing this out. I checked some related discussions and found a summary of various solutions to this issue.

From my perspective, among the proposed solutions, creating a dedicated GitHub App might be better than simply using a repo-scoped PAT. I noticed that currently GH_ACTIONS_REPO_PAT is on your personal account, as shown in previous release PRs. This might be semantically misleading, as the actual performer is the action bot. Moreover, since the account that owns the PAT will be the creator of pull requests, that user account will be unable to perform actions such as requesting changes or approving the pull request.

It doesn't seem complicated to create a GitHub App, as described here. Meanwhile, setting git user information for the app can also be done easily. How about we give this approach a try?

Let me know what you think, thanks!

@hatemhosny
Copy link
Collaborator

Yes, I think this is a reasonable approach.

@zyf722
Copy link
Contributor Author

zyf722 commented Nov 1, 2024

Hi @hatemhosny,

I've just finished with the Github App integration, with some fixes and improvements (changelog):

  • A new Github App has been created here.
    • Its ownership can be transferred to the organization later after changes are merged.
    • We might also want to set it to private afterwards.
  • All i18n workflows and former PAT-based workflows (Release / Deploy) have been modified to use the new Github App.
  • Prevent i18n-update-scheduled from pushing changes from Github to Lokalise, as these source changes should have already been maintained on their own i18n branch.
  • Documentation has been updated to reflect the changes.
  • Some basic testing has been done with i18n workflow (but not with the release / deploy workflows):

Some of my suggestions on next moves after merging:

  • Set CI_APP_ID and CI_APP_PRIVATE_KEY as repo secrets.
  • Install the Github App into the repo.
  • Do some testing with the release / deploy workflows. If everything works as expected, GH_ACTIONS_REPO_PAT could theoretically be removed.
  • Allow GitHub Actions to create and approve pull requests could be revoked in repo settings, as related actions will be handled by the Github App.

Please review these changes. Once you think it's ready, I'll create a PR to merge the changes into the main repo and assist with the next steps proposed above.

Thanks :)

@hatemhosny
Copy link
Collaborator

That's nice indeed, @zyf722
Please go ahead.

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.

2 participants