Bug 240461 – [Commit-Queue] Forbid ChangeLog modification

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

Commit-Queue should only allow commit messages and forbid ChangeLog modification.

[Radar WebKit Bug Importer]

<rdar://problem/93351783>

[Jonathan Bedard]

Created attachment 459431 [details]
Patch

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459431 [details]

[Jonathan Bedard]

Created attachment 459449 [details]
Patch

[Aakash Jain]

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.

[Aakash Jain]

rs=me with these changes and unit-test fixed.

[Jonathan Bedard]

Created attachment 459495 [details]
Patch

[EWS]

Unable to find any modified ChangeLog in Attachment 459495 [details]

[EWS]

Unable to find any modified ChangeLog in Attachment 459495 [details]

[Jonathan Bedard]

https://ews-build.webkit-uat.org/#/builders/26/builds/2095 shows that we are adding the author to the commit message correctly

[Jonathan Bedard]

Created attachment 459496 [details]
Patch

[Aakash Jain]

rs=me

[Jonathan Bedard]

Created attachment 459500 [details]
[fast-cq] Patch

[Jonathan Bedard]

Created attachment 459503 [details]
[fast-cq] Patch

[Jonathan Bedard]

Created attachment 459505 [details]
[fast-cq] Patch

[EWS]

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].

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 459508 [details]
Patch

[EWS]

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].

[Jonathan Bedard]

Re-opening for pull request https://github.com/WebKit/WebKit/pull/732

[Jonathan Bedard]

Pull request: https://github.com/apple/WebKit/pull/7

[EWS]

Committed r294403 (250697@main): <https://commits.webkit.org/250697@main>

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