Bug 240461 - [Commit-Queue] Forbid ChangeLog modification
Summary: [Commit-Queue] Forbid ChangeLog modification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-16 09:46 PDT by Jonathan Bedard
Modified: 2022-05-19 14:13 PDT (History)
3 users (show)

See Also:


Attachments
Patch (7.41 KB, patch)
2022-05-16 09:51 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (15.17 KB, patch)
2022-05-16 12:42 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (29.08 KB, patch)
2022-05-17 06:57 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (30.43 KB, patch)
2022-05-17 07:42 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (33.72 KB, patch)
2022-05-17 08:37 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (31.74 KB, patch)
2022-05-17 09:18 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (31.77 KB, patch)
2022-05-17 09:49 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (2.81 KB, patch)
2022-05-17 11:01 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2022-05-16 09:46:05 PDT
Commit-Queue should only allow commit messages and forbid ChangeLog modification.
Comment 1 Radar WebKit Bug Importer 2022-05-16 09:46:18 PDT
<rdar://problem/93351783>
Comment 2 Jonathan Bedard 2022-05-16 09:51:29 PDT
Created attachment 459431 [details]
Patch
Comment 3 EWS 2022-05-16 10:39:09 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 4 EWS 2022-05-16 11:05:55 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 5 EWS 2022-05-16 11:14:24 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 6 EWS 2022-05-16 12:03:28 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 7 EWS 2022-05-16 12:08:02 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 8 EWS 2022-05-16 12:11:14 PDT
Unable to find any modified ChangeLog in Attachment 459431 [details]
Comment 9 Jonathan Bedard 2022-05-16 12:42:44 PDT
Created attachment 459449 [details]
Patch
Comment 10 Aakash Jain 2022-05-16 16:33:58 PDT
Comment on attachment 459449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=459449&action=review

> Tools/CISupport/ews-build/config.json:338
> +      "workernames": ["webkit-cq-01", "webkit-cq-02", "webkit-cq-03", "ews151"]

seems like this is for debugging

> Tools/CISupport/ews-build/factories.py:309
> +        self.addStep(ShowCommitMessage())

why do we need ShowCommitMessage?

> Tools/CISupport/ews-build/steps.py:882
> +        if not patch:

Should remove this fallback. If the patch is not found, there is something seriously wrong, we shouldn't automatically try to fix that. Better to explicitly fail.

> Tools/CISupport/ews-build/steps.py:893
> +    def hideStepIf(self, results, step):

Nit: should move this next to showStepIf

> Tools/CISupport/ews-build/steps.py:898
> +            return {'step': "Skipping creating commitsince patch_id isn't provided"}

Better not to have custom skipped message, it wouldn't be displayed anyways based on hideStepIf

> Tools/CISupport/ews-build/steps.py:900
> +            return {'step': 'git am failed to apply patch to trunk'}

Nit: "git am" => "git", am is technial details, better to tell user high-level and easy to understand thing.
Comment 11 Aakash Jain 2022-05-17 04:54:19 PDT
rs=me with these changes and unit-test fixed.
Comment 12 Jonathan Bedard 2022-05-17 06:57:27 PDT
Created attachment 459495 [details]
Patch
Comment 13 EWS 2022-05-17 07:03:58 PDT
Unable to find any modified ChangeLog in Attachment 459495 [details]
Comment 14 EWS 2022-05-17 07:07:12 PDT
Unable to find any modified ChangeLog in Attachment 459495 [details]
Comment 15 Jonathan Bedard 2022-05-17 07:08:16 PDT
https://ews-build.webkit-uat.org/#/builders/26/builds/2095 shows that we are adding the author to the commit message correctly
Comment 16 Jonathan Bedard 2022-05-17 07:42:52 PDT
Created attachment 459496 [details]
Patch
Comment 17 Aakash Jain 2022-05-17 08:32:33 PDT
rs=me
Comment 18 Jonathan Bedard 2022-05-17 08:37:13 PDT
Created attachment 459500 [details]
[fast-cq] Patch
Comment 19 Jonathan Bedard 2022-05-17 09:18:07 PDT
Created attachment 459503 [details]
[fast-cq] Patch
Comment 20 Jonathan Bedard 2022-05-17 09:49:35 PDT
Created attachment 459505 [details]
[fast-cq] Patch
Comment 21 EWS 2022-05-17 10:36:36 PDT
Committed r294327 (250647@main): <https://commits.webkit.org/250647@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459505 [details].
Comment 22 Jonathan Bedard 2022-05-17 11:01:21 PDT
Reopening to attach new patch.
Comment 23 Jonathan Bedard 2022-05-17 11:01:22 PDT
Created attachment 459508 [details]
Patch
Comment 24 EWS 2022-05-17 13:12:01 PDT
Committed r294336 (250655@main): <https://commits.webkit.org/250655@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459508 [details].
Comment 25 Jonathan Bedard 2022-05-18 10:08:50 PDT
Re-opening for pull request https://github.com/WebKit/WebKit/pull/732
Comment 26 Jonathan Bedard 2022-05-18 10:17:41 PDT
Pull request: https://github.com/apple/WebKit/pull/7
Comment 27 EWS 2022-05-18 10:27:16 PDT
Committed r294403 (250697@main): <https://commits.webkit.org/250697@main>

Reviewed commits have been landed. Closing PR #732 and removing active labels.