In addition to checking cq+ flag, commit-queue should also check that patch have appropriate review flag (i.e.: r+ or no review flags). Patches with r- or r? should be rejected by commit-queue. This is the same behavior as old EWS.
Created attachment 391548 [details] Patch
Sample runs: https://ews-build.webkit-uat.org/#/builders/26/builds/762 https://ews-build.webkit-uat.org/#/builders/26/builds/750
Comment on attachment 391548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391548&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:390 > + self.setProperty('patch_author', patch_author) This doesn't seem to match the changelog, it looks like it's only patch_author and not commuter and reviewer as well. > Tools/BuildSlaveSupport/ews-build/steps.py:431 > + self.addURL('Reviewed by: {}'.format(patch_reviewer), '') What does 'addURL' do? I don't see a URL in this line.
(In reply to Aakash Jain from comment #2) > Sample runs: > https://ews-build.webkit-uat.org/#/builders/26/builds/762 > https://ews-build.webkit-uat.org/#/builders/26/builds/750 You can review your own patch!?! I've never tried that, I would expect commit queue to reject such a patch.
Comment on attachment 391548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391548&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:390 >> + self.setProperty('patch_author', patch_author) > > This doesn't seem to match the changelog, it looks like it's only patch_author and not commuter and reviewer as well. patch_committer and patch_reviewer are set in subsequent methods. >> Tools/BuildSlaveSupport/ews-build/steps.py:431 >> + self.addURL('Reviewed by: {}'.format(patch_reviewer), '') > > What does 'addURL' do? I don't see a URL in this line. It adds information with a clickable url next to the build step. e.g.: see https://ews-build.webkit-uat.org/#/builders/26/builds/765 We want to display this information (about patch author/reviewer), but don't really need to link to a url. > You can review your own patch!?! > I've never tried that, I would expect commit queue to reject such a patch. Updated code rejects such patches.
Created attachment 391555 [details] Patch
Sample run for rejecting patches r+ed by patch author: https://ews-build.webkit-uat.org/#/builders/26/builds/767
Comment on attachment 391555 [details] Patch Clearing flags on attachment: 391555 Committed r257284: <https://trac.webkit.org/changeset/257284>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59747195>