Mergebot 2 #789
Labels
Comments
This was referenced Jul 4, 2023
Closed
xmo-odoo
added a commit
that referenced
this issue
May 29, 2024
- move all commands parsing to runbot_merge as part of the long-term unification effort (#789) - set up an actual parser-ish structure to parse the commands to something approaching a sum type (fixes #507) - this is mostly prep for reworking the commands set (#673), although *strict command parsing* has been implemented (cf update to `test_unknown_commands`)
xmo-odoo
added a commit
that referenced
this issue
May 29, 2024
This probably has latent bugs, and is only the start of the road to v2 (#789): PR batches are now created up-front (alongside the PR), with PRs attached and detached as needed, hopefully such that things are not broken (tests pass but...), this required a fair number of ajustments to code not taking batches into account, or creating batches on the fly. `PullRequests.blocked` has also been updated to rely on the batch to get its batch-mates, such that it can now be a stored field with the right dependencies. The next step is to better leverage this change: - move cross-PR state up to the batch (e.g. skipchecks, priority, ...) - add fw info to the batch, perform forward-ports batchwise in order to avoid redundant batch-selection work, and allow altering batches during fw (e.g. adding or removing PRs) - use batches to select stagings - maybe expose staging history of a batch?
xmo-odoo
added a commit
that referenced
this issue
Jun 21, 2024
If a PR gets approved *then* fails CI, there should be a notification warning the author & reviewer since 48e08b6, it even has a test, which passes (in fact it has *two*, one of which is redundant, so merge `test_ci_failure_after_review` into the later `test_ci_approved`). *However* this is in runbot_merge, turns out in fafa7ef some refactoring was done in order to override the notification and customise it for *forward ports* with a failed status... except that override never called its `super()`, so as soon as forwardport is installed the base notification stops working, and that's been that since October 2019 (had been added in March that year, ignoring deployment lag). This can be revealed by adding the corresponding check in the *forwardport* tests, revealing the failure. This was a pain to track down, thankfully it reproduced relatively easily locally. While this could be resolved in the override, might as well fold it into the base method in furtherance of #789: the mergebot is only used by odoo, and only with both modules combined, so splitting them is not useful. And furthermore it things should work fine with the forwardport installed but unused. Fixes #894
xmo-odoo
added a commit
that referenced
this issue
Jun 21, 2024
Not actually useful in any way, but it does remove a few lines, avoids a few dupe writes, and furthers the cause of #789
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR batches should be reified and persistent rather than created ad-hoc
The lack of persistent PR batches creates inconsistencies in the shared / cross PR state and makes some features more difficult / ad-hoc than necessary e.g. Support adding new pull requests to an existing forward port #787, or the overall forward port state of a batch.
Probably a rework of the commands system (Rework commands parsing and execution entirely #507, Meta: rework bot commands & interactions #673)
merge the two modules (finally)?
The text was updated successfully, but these errors were encountered: