Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Merge-Queue] Remove custom summaries when skipped #235

Closed
wants to merge 1 commit into from
Closed

[Merge-Queue] Remove custom summaries when skipped #235

wants to merge 1 commit into from

Conversation

JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented Mar 31, 2022 •
edited by webkit-early-warning-system

ddae38d

[Merge-Queue] Remove custom summaries when skipped
https://bugs.webkit.org/show_bug.cgi?id=238633
<rdar://problem/91125435 >

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(ValidateSquashed.getResultSummary):
(AddReviewerToCommitMessage.getResultSummary):
(AddReviewerToChangeLog.getResultSummary):
(ValidateCommitMessage.getResultSummary):
(Canonicalize.getResultSummary):
(PushPullRequestBranch.getResultSummary):
(UpdatePullRequest.getResultSummary):
* Tools/CISupport/ews-build/steps_unittest.py:

Canonical link: https://commits.webkit.org/249189@main



Canonical link: https://commits.webkit.org/249189@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292291 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@JonWBedard JonWBedard self-assigned this Mar 31, 2022
@JonWBedard JonWBedard requested a review from aj062 March 31, 2022 21:36
@JonWBedard JonWBedard added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Nightly Build labels Mar 31, 2022
aj062
aj062 reviewed Mar 31, 2022
View reviewed changes
Tools/CISupport/ews-build/steps.py Outdated
@@ -4638,7 +4638,7 @@ def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):

def getResultSummary(self):
if self.results == SKIPPED:
return {'step': 'Patches are always squashed'}
return super(ValidateSquashed, self).getResultSummary()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the default case.
We should delete if condition for SKIPPED case, and change the current default return Can only land squashed branches to an explicit self.results value (e.g.: if self.results == ERROR).

There are many self.results values which are not covered by current default case (e.g.: WARNINGS, EXCEPTION), complete list here: https://github.com/buildbot/buildbot/blob/master/master/buildbot/process/results.py#L17

@JonWBedard JonWBedard force-pushed the eng/Merge-Queue-Remove-custom-summaries-when-skipped branch from fddb0ac to 3a387ba Compare April 1, 2022 23:49
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Apr 2, 2022
aj062
aj062 approved these changes Apr 4, 2022
View reviewed changes
Tools/CISupport/ews-build/steps.py Outdated
if self.results == SKIPPED:
return {'step': 'Patches are always squashed'}
if self.results == FAILURE:
return {'step': 'Can only land squashed branches'}
elif self.results == SUCCESS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this elif can be if (since previous if statement has a return)

Tools/CISupport/ews-build/steps.py Outdated
if self.results == SKIPPED:
return {'step': 'No reviewer defined' if self.getProperty('github.number') else 'Patches have no commit message'}
if self.results == FAILURE:
return {'step': 'Failed to apply reviewers'}
elif self.results == SUCCESS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Tools/CISupport/ews-build/steps.py Outdated
if self.results == SKIPPED:
return {'step': 'No reviewer defined' if self.getProperty('github.number') else 'Patches are edited upon application'}
if self.results == FAILURE:
return {'step': 'Failed to add reviewers to ChangeLogs'}
elif self.results == SUCCESS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@JonWBedard JonWBedard force-pushed the eng/Merge-Queue-Remove-custom-summaries-when-skipped branch from 3a387ba to 3d55663 Compare April 4, 2022 15:58
@JonWBedard JonWBedard added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Apr 4, 2022
@JonWBedard
[Merge-Queue] Remove custom summaries when skipped
afb86c2 
https://bugs.webkit.org/show_bug.cgi?id=238633
<rdar://problem/91125435>

Patch by Jonathan Bedard <jbedard@apple.com> on 2022-04-04
Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(ValidateSquashed.getResultSummary):
(AddReviewerToCommitMessage.getResultSummary):
(AddReviewerToChangeLog.getResultSummary):
(ValidateCommitMessage.getResultSummary):
(Canonicalize.getResultSummary):
(PushPullRequestBranch.getResultSummary):
(UpdatePullRequest.getResultSummary):
* Tools/CISupport/ews-build/steps_unittest.py:
@webkit-early-warning-system
Copy link
Collaborator

Committed r292291 (?): https://commits.webkit.org/r292291

All reviewed patches have been landed. Closing bug and clearing flags on attachment .

@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Merge-Queue-Remove-custom-summaries-when-skipped branch from 3d55663 to afb86c2 Compare April 4, 2022 16:32
@JonWBedard JonWBedard removed the merge-queue Applied to send a pull request to merge-queue label Apr 4, 2022
@JonWBedard JonWBedard closed this Apr 4, 2022
@JonWBedard JonWBedard deleted the eng/Merge-Queue-Remove-custom-summaries-when-skipped branch April 4, 2022 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aj062 aj062 aj062 approved these changes

Assignees

@JonWBedard JonWBedard

Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Milestone
No milestone
3 participants
@JonWBedard @webkit-early-warning-system @aj062