| Summary: | [ews] add commit-queue build step to clear flags on patch | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||
| Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aakash_jain, ap, jbedard, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=206511 https://bugs.webkit.org/show_bug.cgi?id=207387 |
||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 201934 | ||||||||
| Attachments: |
|
||||||||
|
Description
Aakash Jain
2020-01-21 08:29:47 PST
Created attachment 389843 [details]
WIP
This would also use Bugzilla API key similar to Bug 206511. I think it would be better to store the API key in the passwords.json instead of environment variable as discussed in https://bugs.webkit.org/show_bug.cgi?id=206511#c10 Comment on attachment 389843 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=389843&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:432 > + def _remove_flags_on_patch(self, patch_id): Strikes me as weird that you're having the Mixin own this function. Which actions, other than RemoveFlagsOnPatch, would use it? Comment on attachment 389843 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=389843&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:432 >> + def _remove_flags_on_patch(self, patch_id): > > Strikes me as weird that you're having the Mixin own this function. Which actions, other than RemoveFlagsOnPatch, would use it? Other build-steps like Validating Committer/Reviewer would need to modify the flags (set cq-), which would use very similar logic. I think it's better to keep all the bugzilla logic in one place. Created attachment 390079 [details]
Patch
Sample runs: Success: https://ews-build.webkit-uat.org/#/builders/26/builds/425 Failure: https://ews-build.webkit-uat.org/#/builders/26/builds/423 Comment on attachment 390079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390079&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:534 > + rc = self.remove_flags_on_patch(patch_id) Nit: not sure rc is actually helpful here, I'd probably just put this call inside self.finished. I don't feel particularly strongly about this, though. Comment on attachment 390079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390079&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:534 >> + rc = self.remove_flags_on_patch(patch_id) > > Nit: not sure rc is actually helpful here, I'd probably just put this call inside self.finished. I don't feel particularly strongly about this, though. I like the function call being explicit for better readability, as it's the most important part of this class/method. Committed r256026: <https://trac.webkit.org/changeset/256026> |