Translation bot #12

o-lenczyk · 2022-02-07T13:53:59Z

Resolves #8.

This PR adds:

GitHub Action, that will run every night
bash script that will merge ethereum/solidity into the translated repository and push a commit with merge conflicts that need to be resolved
bash script that will create PR body with a list of changed files in docs directory (as tasks)
instructions on how to set up the Bot and how to create a translation repository

You can see example PR here or here.

workflows/create-pull-request.yaml

chriseth · 2022-02-08T14:45:58Z

workflows/create-pull-request.yaml

+    strategy:
+      fail-fast: false
+      matrix:
+        repos: ["de-german", "fr-french"]


Does it matter in which repository the bot runs? Are there any benefits / drawbacks of running it in the translation repository WRT the origin repository?

One drawback for having it in each repo would be code duplication. Any updates would require updating it in each repo.

Though, on the other hand, this problem would disappear to a large extent it the bot was a reusable GH action. We could have the action in translation-guide repo and other repos just with minimal config using it.

scripts/merge-conflicts.sh

chriseth · 2022-02-08T14:50:41Z

workflows/create-pull-request.yaml

+        with:
+          token: ${{ secrets.PAT }}
+          repository: solidity-docs/translation-guide
+          path: .github-workflow


Can you explain this?

Looks like the bot uses a Personal Access Token. The repos it's accessing are all public so it's not needed just for reading. For pushing and creating PRs the action might actually need to use bot's token but I'd try with secrets.GITHUB_TOKEN first and only go for PAT if that's proven not to be enough.

@chriseth
Yes, indeed, this part is not really intuitive. I need to fetch the repository for external shell scripts. The flow will look like this:

fetch translation repository

fetch merge scripts (from the same repository as GitHub Action)

fetch original repository and merge it to translation repository

publish sync branch with a merge commit

create a pull request

@cameel
I am using Bot Personal Access Token. I wanted the PR to be created by customized "Solidity PR Bot" and not generic "GitHub Action". Is this ok?

As we agreed on the call, it should work both with and without a dedicated account for the bot. We'll probably use it without an account but we can keep an option open to start using one later.

chriseth · 2022-02-08T14:51:58Z

workflows/create-pull-request.yaml

+      - name: Create merge commit
+        id: merge
+        run: |
+          ./.github-workflow/scripts/merge-conflicts.sh


Is there a benefit in having two script files?

I think two scripts are fine but what I don't like about it is that input an output is passed implicitly through shell env. I think the scripts should take input through parameters and I'd also make the second script just print the body to the output, without being aware that it's running in CI. I.e.:

.github-workflow/scripts/merge-conflicts.sh "$BOT_USERNAME" "$BOT_EMAIL" title="Sync with ethereum/solidity@develop $(date +%d-%m-%Y)" body="$(.github-workflow/scripts/generate-PR-body.sh)" printf "pr_title=%s\n" "$(title)" >> "$GITHUB_ENV" printf "pr_body<<EOF\n%s\nEOF\n" "$(body)" >> "$GITHUB_ENV"

yes, I fixed inputs, but right now I do not see a clear way how to fix outputs. GITHUB_ENV and echo ::set-output are the only ways that I found.

GITHUB_ENV is fine, it's just that I'd keep it in yaml and pass data from the script in a more CI-agnostic way.

I think I do not get it. peter-evans/create-pull-request@v3 action is accepting strings, not yaml files.

By "in yaml" I meant in create-pull-request.yaml. I.e. keep it out of the Bash script. See my snippet a few comments above. It shows exactly what I meant.

In any case, this is not critical. I'd change it but I'm still going to approve if you prefer it as is.

workflows/create-pull-request.yaml

cameel · 2022-02-08T15:18:00Z

workflows/create-pull-request.yaml

+    strategy:
+      fail-fast: false
+      matrix:
+        repos: ["de-german", "fr-french"]


One drawback for having it in each repo would be code duplication. Any updates would require updating it in each repo.

Though, on the other hand, this problem would disappear to a large extent it the bot was a reusable GH action. We could have the action in translation-guide repo and other repos just with minimal config using it.

cameel · 2022-02-08T15:28:36Z

workflows/create-pull-request.yaml

+        with:
+          token: ${{ secrets.PAT }}
+          repository: solidity-docs/translation-guide
+          path: .github-workflow


Looks like the bot uses a Personal Access Token. The repos it's accessing are all public so it's not needed just for reading. For pushing and creating PRs the action might actually need to use bot's token but I'd try with secrets.GITHUB_TOKEN first and only go for PAT if that's proven not to be enough.

workflows/create-pull-request.yaml

scripts/merge-conflicts.sh

cameel · 2022-02-16T17:44:36Z

@o-lenczyk Can you push your changes? I see that you marked many comments as resolved but they're not yet resolved in the code - it's still the same code I reviewed last time.

o-lenczyk · 2022-02-17T09:04:31Z

@o-lenczyk Can you push your changes? I see that you marked many comments as resolved but they're not yet resolved in the code - it's still the same code I reviewed last time.

yup, I wanted to push everything at once

scripts/merge-conflicts.sh

scripts/set-assignees.sh

.github/workflows/create-daily-docs-sync-pr.yaml

scripts/merge-conflicts.sh

.github/workflows/create-daily-docs-sync-pr.yaml

scripts/set-assignees.sh

cameel

There are only a few small issues remaining:

Should work with and without PAT as agreed on the call
Include last tag in PR title and commit description as agreed on the call
Minor quoting issues (see unresolved comments)

I'd like to get this merged soon so I'd be also fine with leaving (1) and (2) for upcoming PRs as long as you create issues so that we don't forget about them. Though I think (2) is minor enough to still do it here.

scripts/set-assignees.sh

scripts/merge-conflicts.sh

.github/workflows/create-daily-docs-sync-pr.yaml

cameel · 2022-03-07T19:42:09Z

scripts/generate-PR-body.sh

+set -euo pipefail
+
+function title {
+    echo "Sync with ethereum/solidity@develop $(date +%d-%m-%Y)"


Can you include the tag in PR title (and commit description)?

On the call we agreed to leave labels for #15 but the title change is simpler so it was meant to be still applied here.

will this be ok?

"Sync with ethereum/solidity@$(git describe --tags --always english/develop) $(date +%d-%m-%Y)"

I see that it looks like this in https://github.com/solidity-docs-test/pl-polish/pull/60:

Sync with ethereum/solidity@v0.8.12-88-ga5a65eec9 09-03-2022 (PR #60)

I think it's good.

At least for the purposes of this PR it's exactly what I wanted and we can tweak it later if necessary. It's a bit crowded with details now but all the details are useful in some way so I'm not even sure we can really improve on that.

After the tests, I think putting the most recent tag in PR title can be misleading. The source of the PR is always the most recent commit, the most recent tag is not always the most recent revision.

Oh, you mean that the last tag is e.g. 0.8.13 while the commits now will be a part of 0.8.14? That's true, but getting that is (slightly) more complicated. You need to run scripts/get_version.sh to get it.

But we still need some way to separate PRs for one version from another. If you can run that script to get the actual version, great. If not, I think that the version from the tag is better than nothing.

cameel

I'm marking as approved since this looks ready in terms of functionality.

One last change before we merge though is to disable it for now so that it does not run on existing repos before we finish testing in the test org and before the repos are converted.

Would be good for @chriseth to take a final look at this.

.github/workflows/README.md

.github/workflows/create-daily-docs-sync-pr.yaml

cameel · 2022-03-09T16:51:21Z

Also, please mark the PR as non-draft.

scripts/generate-PR-body.sh

README.md

.github/workflows/create-daily-docs-sync-pr.yaml

cameel

I suggested some final tweaks but PR seems fine technically. We should re-enable the bot somewhere though to have a final look at the PRs it creates.

The README probably will need to be adjusted to contain only info relevant to translators (perhaps we should have a separate file for details of bot management that are only relevant to us) but I'm assuming we'll do that with @franzihei once the bot is up and running.

Also, now that repos are actually migrated, we should add the ones that are ready to the matrix. I originally assumed we'd be merging before we launch but now it looks like the merge will be the launch.

scripts/merge-conflicts.sh

.github/workflows/README.md

cameel · 2022-07-08T14:57:58Z

The PR only needed small tweaks and could be merged but I cannot push to the branch here so I had to create a new one: #23. This one is now redundant so I'm closing it.

o-lenczyk changed the title ~~#8 Translation bot~~ Translation bot Feb 7, 2022

franzihei mentioned this pull request Feb 8, 2022

Repo Automation: Diff Bot for Translation Updates #8

Closed

franzihei requested a review from chriseth February 8, 2022 11:40

chriseth reviewed Feb 8, 2022

View reviewed changes

workflows/create-pull-request.yaml Outdated Show resolved Hide resolved

chriseth reviewed Feb 8, 2022

View reviewed changes

scripts/merge-conflicts.sh Outdated Show resolved Hide resolved

chriseth reviewed Feb 8, 2022

View reviewed changes

cameel reviewed Feb 8, 2022

View reviewed changes

cameel reviewed Feb 18, 2022

View reviewed changes

This was referenced Mar 1, 2022

Automate creation of new translation repositories #16

Open

Dynamic loading of the list of available translations #17

Open

cameel reviewed Mar 7, 2022

View reviewed changes

cameel approved these changes Mar 9, 2022

View reviewed changes

.github/workflows/README.md Outdated Show resolved Hide resolved

.github/workflows/README.md Outdated Show resolved Hide resolved

.github/workflows/create-daily-docs-sync-pr.yaml Outdated Show resolved Hide resolved

chriseth reviewed Mar 9, 2022

View reviewed changes

scripts/generate-PR-body.sh Outdated Show resolved Hide resolved

cameel reviewed Mar 15, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

franzihei reviewed May 30, 2022

View reviewed changes

.github/workflows/create-daily-docs-sync-pr.yaml Outdated Show resolved Hide resolved

.github/workflows/create-daily-docs-sync-pr.yaml Outdated Show resolved Hide resolved

o-lenczyk added 4 commits June 6, 2022 00:05

Translation bot: add GitHub Action

08f3769

Translation bot: add scripts

d0f10b7

Translation bot: add documentation

6873355

Translation bot: do not create PR if sync branch exists

bf42992

cameel reviewed Jun 7, 2022

View reviewed changes

scripts/merge-conflicts.sh Show resolved Hide resolved

scripts/merge-conflicts.sh Show resolved Hide resolved

.github/workflows/README.md Show resolved Hide resolved

cameel mentioned this pull request Jul 8, 2022

Translation bot (with tweaks) #23

Merged

cameel closed this Jul 8, 2022

cameel mentioned this pull request Dec 20, 2022

Replace deprecated set-output with writing to $GITHUB_OUTPUT #40

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Translation bot #12

Translation bot #12

o-lenczyk commented Feb 7, 2022 •

edited by cameel

Loading

chriseth Feb 8, 2022

cameel Feb 8, 2022

chriseth Feb 8, 2022

cameel Feb 8, 2022 •

edited

Loading

o-lenczyk Feb 13, 2022 •

edited

Loading

o-lenczyk Feb 25, 2022

cameel Mar 7, 2022

chriseth Feb 8, 2022

cameel Feb 8, 2022

o-lenczyk Feb 13, 2022

cameel Feb 16, 2022

o-lenczyk Feb 25, 2022

cameel Mar 7, 2022

cameel Feb 8, 2022

cameel Feb 8, 2022 •

edited

Loading

cameel commented Feb 16, 2022 •

edited

Loading

o-lenczyk commented Feb 17, 2022 •

edited

Loading

cameel left a comment •

edited

Loading

cameel Mar 7, 2022

o-lenczyk Mar 8, 2022

cameel Mar 9, 2022

o-lenczyk Apr 1, 2022

cameel Apr 7, 2022 •

edited

Loading

cameel left a comment

cameel commented Mar 9, 2022

cameel left a comment

cameel commented Jul 8, 2022

Translation bot #12

Translation bot #12

Conversation

o-lenczyk commented Feb 7, 2022 • edited by cameel Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameel Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

o-lenczyk Feb 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameel Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

cameel commented Feb 16, 2022 • edited Loading

o-lenczyk commented Feb 17, 2022 • edited Loading

cameel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameel Apr 7, 2022 • edited Loading

Choose a reason for hiding this comment

cameel left a comment

Choose a reason for hiding this comment

cameel commented Mar 9, 2022

cameel left a comment

Choose a reason for hiding this comment

cameel commented Jul 8, 2022

o-lenczyk commented Feb 7, 2022 •

edited by cameel

Loading

cameel Feb 8, 2022 •

edited

Loading

o-lenczyk Feb 13, 2022 •

edited

Loading

cameel Feb 8, 2022 •

edited

Loading

cameel commented Feb 16, 2022 •

edited

Loading

o-lenczyk commented Feb 17, 2022 •

edited

Loading

cameel left a comment •

edited

Loading

cameel Apr 7, 2022 •

edited

Loading