Skip to content

CI: branch collision handling for translation templates updates#7128

Merged
echoix merged 3 commits intoOSGeo:mainfrom
Dasux:fix-branch-collision
Mar 12, 2026
Merged

CI: branch collision handling for translation templates updates#7128
echoix merged 3 commits intoOSGeo:mainfrom
Dasux:fix-branch-collision

Conversation

@Dasux
Copy link
Copy Markdown
Contributor

@Dasux Dasux commented Mar 1, 2026

This PR resolves the issuue #7115 ... related to the update-i18n job in the Periodic update workflow.

Previously, the workflow used a static branch name:

periodic/update-i18n

This meant that every time the workflow ran, it reused the same branch to create or update a Pull Request. As a result, only one translation update PR could exist at a time. If the workflow was triggered again (either by schedule or manually) while a previous PR was still open, the same branch would be reused and the existing PR would be updated or overwritten.

What was changed

The branch name used by the create-pull-request action has been updated to include a unique identifier:

periodic/update-i18n-${{ github.run_id }}

so this ensures that

  • each workflow run creates it's own branch
  • multiple translation PRs can coexist at the same time... without interfering with one another
  • no existing PR is unintentionally overwritten

Verification

The workflow was manually triggered manually on a fork repo, and the branches were different each time. Each run created a distinct branch and corresponding Pull Request, confirming that branch names no longer collide.

image image

Note:

Assistance from AI was used for understanding the issue and hinting towards a solution

@github-actions github-actions bot added the CI Continuous integration label Mar 1, 2026
Copy link
Copy Markdown
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Can you review the diff, and only changes the lines needed? You removed lots of comments, changed comments where not needed, in short, it’s hiding the real useful changes.

Add yourself in the loop, like we wrote in the contribution guidelines/ai policy, and come ping me back when it will be worth it :)

Your solution with the run_id is probably better than using the ref/base_ref (that was previously mentionnend), as it it less prone to injection, so a bit better on security.

Comment thread .github/workflows/docker.yml
Copy link
Copy Markdown
Contributor Author

@Dasux Dasux left a comment

Choose a reason for hiding this comment

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

i reverted docker.yml

Copy link
Copy Markdown
Contributor Author

@Dasux Dasux left a comment

Choose a reason for hiding this comment

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

  1. merged the latest main into the branch and resolved the conflicts...
  2. restored the removed comments and cleaned up the workflow so the diff only reflects the intended changes.

@Dasux Dasux requested a review from echoix March 9, 2026 14:10
@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 9, 2026

Also, as a rule of thumb moving forward, how should PRs be titled?
I’ve noticed one workflow occasionally fails when the PR title doesn’t match the repository’s conventions.

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 9, 2026

Take a look again at what I have to review, it doesn't seem humanly reviewed-reread

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 9, 2026

Also, as a rule of thumb moving forward, how should PRs be titled?

I’ve noticed one workflow occasionally fails when the PR title doesn’t match the repository’s conventions.

Look at the other (merged) PRs, and the output of the failed workflow. It checks for some known categories, and shows the allowed ones.

Read again https://github.com/OSGeo/grass/blob/main/CONTRIBUTING.md and also the part where it links to https://github.com/OSGeo/grass/blob/main/doc/development/github_guide.md#pr-title

@Dasux Dasux changed the title Fix branch collision -- issue #7115 CI: branch collision handling -- issue #7115 Mar 9, 2026
revert: reverted unintended changes to docker workflow
@Dasux Dasux force-pushed the fix-branch-collision branch from dfd9c84 to c60f14a Compare March 9, 2026 20:44
@github-actions github-actions bot added windows Microsoft Windows specific macOS macOS specific docker Docker related raster Related to raster data processing Python Related code is in Python C Related code is in C translation Message translation related libraries module docs imagery nix labels Mar 9, 2026
@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 9, 2026

Bad rebase

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 9, 2026

Either you are rebasing with a main that is behind, like your fork’s main branch, you are not using upstream/main (assuming the remote for this upstream repo is called upstream in your local env).

It’s easier when there’s no force pushes yet, but to get out of it, you need to check one of both and do another force-push (which I say again, is not pleasant when reviewing has started)

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 9, 2026

very bad rebase..... i know...

ill go back to the previous commit hash---

@Dasux Dasux force-pushed the fix-branch-collision branch from c60f14a to 61f843f Compare March 9, 2026 20:56
Copy link
Copy Markdown
Contributor Author

@Dasux Dasux left a comment

Choose a reason for hiding this comment

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

hopefully now the diff shows only the intended changes ....

@echoix echoix removed the windows Microsoft Windows specific label Mar 10, 2026
@echoix echoix removed macOS macOS specific docker Docker related raster Related to raster data processing Python Related code is in Python C Related code is in C libraries module docs imagery nix labels Mar 10, 2026
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
Comment thread .github/workflows/periodic_update.yml Outdated
@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 10, 2026

hopefully now the diff shows only the intended changes ....

Thanks, now reviewing was almost limited to two chunks, so way easier!

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 10, 2026

I removed the concurrency part...

Also..

hopefully now the diff shows only the intended changes ....

Thanks, now reviewing was almost limited to two chunks, so way easier!

thank you for being patient and guiding me along the way
really appreciate it...

@echoix echoix changed the title CI: branch collision handling -- issue #7115 CI: branch collision handling for translation templates updates Mar 10, 2026
Dasux

This comment was marked as resolved.

@Dasux Dasux requested a review from echoix March 11, 2026 11:03
@echoix echoix enabled auto-merge (squash) March 11, 2026 13:06
@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 11, 2026

You just need to resolve the review comments, some might be hidden somewhere because of the force-pushes

@echoix echoix merged commit 879f381 into OSGeo:main Mar 12, 2026
28 checks passed
@github-actions github-actions bot added this to the 8.6.0 milestone Mar 12, 2026
@echoix echoix added the backport to 8.5 PR needs to be backported to release branch 8.5 label Mar 17, 2026
@neteler neteler removed the backport to 8.5 PR needs to be backported to release branch 8.5 label Apr 17, 2026
@neteler neteler modified the milestones: 8.6.0, 8.5.0 Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration translation Message translation related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants