| Summary: | [ews] commit-queue should check that patch have appropriate review flag | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||
| Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aakash_jain, ap, commit-queue, jbedard, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=209231 | ||||||||
| Attachments: |
|
||||||||
|
Description
Aakash Jain
2020-02-24 08:33:01 PST
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. |