Skip to content

[#72881] Handle generated Crowdin conflicts in release-to-dev merges#22249

Open
myabc wants to merge 1 commit intodevfrom
code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution
Open

[#72881] Handle generated Crowdin conflicts in release-to-dev merges#22249
myabc wants to merge 1 commit intodevfrom
code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution

Conversation

@myabc
Copy link
Copy Markdown
Contributor

@myabc myabc commented Mar 7, 2026

Ticket

https://community.openproject.org/wp/72881

Summary

Goal: Reduce manual conflict resolution work when create-merge-release-into-dev-pr opens a PR for merging the latest release/* branch into dev.

This adds a helper that handles generated Crowdin locale conflicts on the temporary merge-release/... branch and wires it into the workflow. It also documents the same helper for developers resolving those branches locally.

Why this approach

Although Crowdin locale files are generated artifacts, they are generated independently on dev and release branches. A three-way merge preserves non-overlapping generated changes without blindly taking one side when both branches changed the same translation entry differently. Later Crowdin runs can still normalize the files, but this reduces manual conflict work without discarding valid branch-specific generated output.

.gitattributes would apply globally, but this behavior is specific to the release-to-dev maintenance workflow. Keeping it in the workflow/helper makes the automation targeted and easier to reason about.

What changed

  • added OpenProject::GeneratedLocaleConflictMerger
  • added script/i18n/merge_generated_locale_conflicts
  • updated create-merge-release-into-dev-pr to run the helper on the PR branch path
  • documented the local conflict-resolution workflow
  • added focused specs for merge behavior, symbol-valued YAML, formatting preservation, and safe deletions

Merge behavior

The helper only touches unresolved files matching **/config/locales/crowdin/*.yml (i.e. config/locales/crowdin/*.yml and modules/*/config/locales/crowdin/*.yml).

It:

  • parses merge stages as YAML
  • recursively merges hash-shaped locale data
  • permits symbol-valued YAML entries used by locale files
  • prefers the dev side when both branches changed the same leaf differently
  • preserves the raw YAML from a merge stage when the resolved content already matches that stage — to avoid reserialization churn, files whose merged YAML differs from all raw stages are left unresolved
  • stages git rm when deletion is the correct safe result

Only expected merge failures (invalid YAML, non-hash top-level, no matching raw stage) are caught via MergeError. Git plumbing errors and other unexpected failures propagate as real errors rather than being silently treated as unresolved files.

Files outside the generated Crowdin locale paths are left alone. If conflicts remain after running the helper, they still require manual resolution.

Workflow integration

After git merge fails, the workflow follows one of four paths:

  1. Script exits 0, no remaining conflicts — all conflicts (generated and otherwise) are resolved; the merge is committed with a NOTE explaining the auto-resolution
  2. Script exits 0, non-generated conflicts remain — generated locale conflicts were resolved but other files still conflict; the merge is aborted and the PR is opened with a WARNING directing the developer to resolve the remaining conflicts and rerun the helper
  3. Script exits 2 — some generated locale conflicts could not be auto-resolved; the merge is aborted and the PR is opened with a WARNING
  4. Script exits with any other code — unexpected error (e.g. git plumbing failure); the workflow fails immediately

Merge checklist

Copilot AI review requested due to automatic review settings March 7, 2026 20:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an automated, conservative 3-way YAML merge helper to reduce manual conflict resolution when release→dev merges conflict only in generated Crowdin locale files, and integrates it into the existing release-to-dev merge PR workflow and docs.

Changes:

  • Introduce OpenProject::GeneratedLocaleConflictMerger with safe 3-way YAML merge logic for generated Crowdin locale conflicts.
  • Add a runnable helper script (script/i18n/merge_generated_locale_conflicts) and RSpec coverage for common conflict scenarios.
  • Update the create-merge-release-into-dev-pr workflow and developer docs to use/describe the helper during merge-release/... conflict resolution.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/open_project/generated_locale_conflict_merger.rb Implements conservative 3-way YAML merge for conflicted generated locale files and stages safe resolutions.
script/i18n/merge_generated_locale_conflicts Adds a CLI entrypoint to run the merger locally/CI.
spec/lib/open_project/generated_locale_conflict_merger_spec.rb Covers merge behavior for key scenarios (one-side change, nested merge, scalar conflict, invalid YAML, missing stages, mixed conflicts).
docs/development/git-workflow/README.md Documents how to use the helper when resolving merge-release/... branches locally.
.github/workflows/create-merge-release-into-dev-pr.yml Runs the helper during merge-conflict handling and annotates PR body with outcome.

You can also share your feedback on Copilot code review. Take the survey.

@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from 36da46e to bf214c5 Compare March 7, 2026 21:09
@myabc myabc added maintenance needs review ruby Pull requests that update Ruby code labels Mar 7, 2026
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from bf214c5 to feb4575 Compare March 7, 2026 21:15
@myabc myabc changed the title Handle generated Crowdin conflicts in release-to-dev merges [#72881] [#72881] Handle generated Crowdin conflicts in release-to-dev merges Mar 7, 2026
@myabc myabc requested review from a team and Copilot March 7, 2026 21:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.


You can also share your feedback on Copilot code review. Take the survey.

@myabc myabc added the ci label Mar 8, 2026
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch 2 times, most recently from 9058168 to c746cc7 Compare March 8, 2026 16:43
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from 2d1f69a to 07ea65e Compare March 8, 2026 16:55
Copy link
Copy Markdown
Contributor

@NobodysNightmare NobodysNightmare left a comment

Choose a reason for hiding this comment

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

I like the new version of the code much more. By now I might be biased, because I read it once before, but it was much easier for me to get into the Merger class and navigate between methods when I had questions about how something worked.

I left some additional feedback, but nothing blocking from my side.

@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch 3 times, most recently from 52bf929 to 8991582 Compare March 9, 2026 14:34
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from 8991582 to 735bb76 Compare March 10, 2026 17:44
@myabc myabc requested a review from Copilot March 10, 2026 17:51
@myabc myabc requested a review from toy March 10, 2026 17:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.


You can also share your feedback on Copilot code review. Take the survey.

@myabc myabc requested review from cbliard and toy March 11, 2026 09:24
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from b36c094 to 945b7cd Compare April 3, 2026 17:20
@myabc myabc requested a review from Copilot April 3, 2026 17:20
@myabc myabc assigned toy Apr 3, 2026
@myabc
Copy link
Copy Markdown
Contributor Author

myabc commented Apr 3, 2026

@toy I've tried to address all your comments. Please take another look!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Adds `GeneratedLocaleConflictMerger` to handle generated Crowdin
locale conflicts in the `create-merge-release-into-dev-pr` workflow.

The helper reads Git merge stages, performs a recursive three-way
YAML merge for hash-shaped locale data under crowdin paths, and
prefers the dev side when both branches changed the same leaf.
Files are left unresolved when the merged result differs from all
raw stages (avoiding YAML reserialization churn), or when YAML
cannot be parsed safely.

Wires the helper into the workflow so that locale-only conflicts
are resolved automatically. When non-locale conflicts remain, the
merge is aborted and the PR body notes manual resolution is needed.

Adds a `script/i18n/merge_generated_locale_conflicts` entry point
for local use and documents the workflow in the git-workflow README.
@myabc myabc force-pushed the code-maintenance/72881-merge-release-into-dev-pr-crowdin-resolution branch from 945b7cd to e8d4069 Compare April 3, 2026 17:42
@myabc
Copy link
Copy Markdown
Contributor Author

myabc commented Apr 3, 2026

@toy some discussion points before final review/merge:

  • the PR does add value: we've now used it on several actual merge-release/... branches, and it successfully auto-resolved generated Crowdin conflicts while staying tightly scoped to **/config/locales/crowdin/*.yml.

  • but it's not perfect: despite a lot of timetokens spent on this solution, it might be more sophisticated than our real workflow needs. In practice, for these release-to-dev merges, we usually want take dev for generated Crowdin files. The current recursive YAML merge logic can still leave edge cases unresolved.

tl;dr I think this a step in the right direction, but I think we should revisit and possibly simplify.

@toy
Copy link
Copy Markdown
Contributor

toy commented Apr 8, 2026

@myabc I think it is better to first merge #22519 and simplify the auto merging script before merging this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci maintenance needs review ruby Pull requests that update Ruby code

Development

Successfully merging this pull request may close these issues.

5 participants