RESOLVED FIXED 283086
unsafe-merge-queue not rewriting "(OOPS!)" with "Originally-landed-as"
https://bugs.webkit.org/show_bug.cgi?id=283086
Summary unsafe-merge-queue not rewriting "(OOPS!)" with "Originally-landed-as"
Sam Sneddon [:gsnedders]
Reported 2024-11-13 15:25:30 PST
Trying to land https://github.com/WebKit/WebKit/pull/36565, where the commit is applied from https://github.com/bugzilla/harmony/commit/effcc512d, albeit with a completely rewritten commit message, failed in the unsafe-merge-queue because it contained "(OOPS!)". Notably, this ends with the trailer "Originally-landed-as: https://github.com/bugzilla/harmony/commit/effcc512d". Now, looking at Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/trace.py will tell you that this probably not the intended format of "Originally-landed-as", but it seemed not entirely silly. (AIUI, the intended semantic of "Originally-landed-as" is commits that don't _start_ on main (and are being merged to another branch, potentially main), but do start within the same commit graph.) The only error https://ews-build.webkit.org/#/builders/22/builds/11808 gives is from validate-commit-message, "Commit message contains (OOPS!)". With AddReviewerToCommitMessage having https://github.com/WebKit/WebKit/blob/e92bf171d84d991d69c755f872a08a3d1e13d2ed/Tools/CISupport/ews-build/steps.py#L6689-L6690: ``` def hideStepIf(self, results, step): return not self.doStepIf(step) ``` There was nothing to indicate _why_ it hadn't added reviewers. While this is potentially niche, given commits with Originally-landed-as have been disproportionately landed by Robert Jenner (60%) and Jonathan Bedard (34%), it nevertheless hides things that might be useful for the 6% of the time when other people make commits with Originally-landed-as trailers.
Attachments
Radar WebKit Bug Importer
Comment 1 2024-11-13 15:25:41 PST
Brianna Fan
Comment 2 2025-01-28 10:47:30 PST
EWS
Comment 3 2025-02-11 09:26:52 PST
Committed 290215@main (cc1b3c8bf6c2): <https://commits.webkit.org/290215@main> Reviewed commits have been landed. Closing PR #39633 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.