Skip to content

[canary] 🚀 Use dataclasses for 3way merging #28160

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

Merged
merged 1 commit into from
Mar 18, 2025

Conversation

cdesouza-chromium
Copy link
Collaborator

@cdesouza-chromium cdesouza-chromium commented Mar 16, 2025

This PR introduces several data classes to better compartimentalise the data being used in this tool. The previous code relied on a very large method to do all the operations necessary during patch resolution.

The management of the continuation file is also being overhauled to be its own code, separate from PatchFailureResolver, as the continuation file also stores things like working_version which is not necessarily part of the merge process.

Version Type
This type is being introduced to simplify the operations around version in Brockit. The code is now easier to understand, and the operations are grouped.

Infra Mode
This new execution mode suppresses the status page, and as a replacement, provides terminal "keep alive" output, to allow the CI to be kept alive.

A separate issue has been created to come up with something better than pickle for serialisation brave/brave-browser#44726

Resolves brave/brave-browser#44690
Resolves brave/brave-browser#44727

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@cdesouza-chromium cdesouza-chromium added the CI/skip Do not run CI builds (except noplatform) label Mar 16, 2025
@cdesouza-chromium cdesouza-chromium self-assigned this Mar 16, 2025
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch 2 times, most recently from 5bcad87 to 6e36028 Compare March 16, 2025 05:22
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch from 6e36028 to bb1aba8 Compare March 16, 2025 05:33
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch 2 times, most recently from 5d561f8 to 3455342 Compare March 17, 2025 17:15
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch from 3455342 to 13ff5dd Compare March 17, 2025 22:47
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch 2 times, most recently from 2503158 to 68d0d12 Compare March 17, 2025 23:17
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch 2 times, most recently from 88c53dd to 4095d40 Compare March 18, 2025 00:26
This PR introduces several data classes to better compartimentalise the
data being used in this tool. The previous code relied on a very large
method to do all the operations necessary during patch resolution.

The management of the continuation file is also being overhauled to be
its own code, separate from `PatchFailureResolver`, as the continuation
file also stores things like `working_version` which is not necessarily
part of the merge process.

**Version Type**

This type is being introduced to simplify the operations around version
in Brockit. The code is now easier to understand, and the operations are
grouped.

**Infra Mode**
This new execution mode suppresses the status page, and as a
replacement, provides terminal "keep alive" output, to allow the CI to
be kept alive.

Resolves brave/brave-browser#44690
Resolves brave/brave-browser#44727
@cdesouza-chromium cdesouza-chromium force-pushed the brockit-dataclasses-during-3way branch from 4095d40 to 2293f5c Compare March 18, 2025 17:04
@cdesouza-chromium cdesouza-chromium merged commit c611155 into master Mar 18, 2025
18 checks passed
@cdesouza-chromium cdesouza-chromium deleted the brockit-dataclasses-during-3way branch March 18, 2025 17:19
@github-actions github-actions bot added this to the 1.78.x - Nightly milestone Mar 18, 2025
@brave-builds
Copy link
Collaborator

Released in v1.78.51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip Do not run CI builds (except noplatform) needs-security-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Add infra mode to satisfy CI keep-alive requirements 🚀 Break apart patch resolution into smaller parts
5 participants