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

Modify sync-pr to be triggered on Solidity release #53

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

r0qs
Copy link
Contributor

@r0qs r0qs commented Feb 6, 2024

This PR modifies the behavior of the sync-pr bot. While the workflow still triggers daily to monitor the latest Solidity release, it will now only initiate sync-prs if a new release is detected. The new job compares the latest tagged release with the current release stored in this repository (initially set to 0.8.24). If they differ, it will then trigger the createPullRequest job to generate the sync PRs on the translation repositories and create another PR to this repo to update the current version to the latest release.

Fixes #22

@r0qs r0qs force-pushed the sync-pr-bot-on-release branch from b866587 to c7c31e7 Compare February 6, 2024 10:15
@r0qs r0qs marked this pull request as ready for review February 6, 2024 10:16
@r0qs r0qs requested review from franzihei and cameel February 6, 2024 10:19
@r0qs r0qs changed the title Triggers sync-pr on Solidity release Modify sync-pr to be triggered on Solidity release Feb 6, 2024
@@ -36,7 +58,7 @@ jobs:
- zh-chinese
steps:
- name: Fetch translation repository
uses: actions/checkout@v2
uses: actions/checkout@v4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the PR depend on this? It seems like it's an independent change so it should be at least a separate commit. Or even a separate PR.

@@ -127,3 +149,25 @@ jobs:
labels: "${{ join(fromJSON(steps.bot-config.outputs.pr_labels)) }}"
assignees: ${{ env.assignee }}
reviewers: ${{ env.reviewer }}

- name: Fetch Solidity translation repository
uses: actions/checkout@v4
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fetch bot's repository, not the translation's repository.

createPullRequest:
runs-on: ubuntu-latest
needs: checkSolidityRelease
if: needs.checkSolidityRelease.outputs.current_release != needs.checkSolidityRelease.outputs.latest_release
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not quite what I meant in #22. This will still create a daily sync PR, just at some arbitrary moment after the release. That's because pull-and-resolve-english-changes.sh will still pull code from develop and not the tag so it may have some extra commits.

Doing it properly, on the tag, isn't even really harder if you're replacing the daily sync PRs (rather than keeping support for both).

createPullRequest:
runs-on: ubuntu-latest
needs: checkSolidityRelease
if: needs.checkSolidityRelease.outputs.current_release != needs.checkSolidityRelease.outputs.latest_release
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another problem is that it can only create a PR for the latest release. We should really create a PR for the first untagged release in the repo and only when there's no other pending release sync PR (we can make that no pending sync PR in general for simplicity). So that translators can translate one version at a time. We could fall back to latest if the translation repo has no version tags though.

Doing it this way would actually be simpler, because you'd not need this whole stateful setup with keeping a version file in the repo and having to create PRs to update it - which will just result in more maintenance effort for us.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The sync PR description also says that it's perfectly safe to close a sync PR and wait for the next one and with this change it's no longer true, which will create problems for translators.

The description of the sync PR should also be updated to say that sync PRs are generated when there's an untranslated release and no pending sync PR (rather than in regular intervals as until now) so that they know when to expect one.

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.

Translation bot: sync PRs for tagged releases
2 participants