Summary: | [ews] Resubmit patches to commit-queue which were cq- by commit-queue and later cq+ | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
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=196628 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 201934 | ||||||||||
Attachments: |
|
Description
Aakash Jain
2020-03-11 08:56:38 PDT
Created attachment 393247 [details]
WIP
Created attachment 393778 [details]
Patch
Comment on attachment 393778 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393778&action=review > Tools/BuildSlaveSupport/ews-app/ews/fetcher.py:106 > def patches_to_send_to_buildbot(self, patch_ids, commit_queue=False): If we are adding a new 'patches_to_send_to_commit_queue' function, what's the point of this flag? > Tools/BuildSlaveSupport/ews-app/ews/common/bugzilla.py:67 > + def convert_iso_timestamp(cls, iso_timestamp): Nit: Not convinced that this should be a separate function, I would have put the logic in the above function. > Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:-102 > - return Buildbot.icons_for_queues_mapping I think this deletion was unintentional. > Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:104 > + def fetch_builders(cls): Do we ever intend to call this outside of update_builder_name_to_id_mapping? Seems like we should consider putting the contents inside that function Created attachment 393788 [details]
Patch
(In reply to Jonathan Bedard from comment #3) > If we are adding a new 'patches_to_send_to_commit_queue' function, what's the point of this flag? Removed in updated patch > Nit: Not convinced that this should be a separate function, I would have put the logic in the above function. Moved inside same function. > > - return Buildbot.icons_for_queues_mapping > I think this deletion was unintentional. It was actually intentional. The return value is never used and the function name also doesn't indicate that the function will return anything. > Do we ever intend to call this outside of update_builder_name_to_id_mapping? Seems like we should consider putting the contents inside that function Done in updated patch. Committed r258611: <https://trac.webkit.org/changeset/258611> All reviewed patches have been landed. Closing bug and clearing flags on attachment 393788 [details]. |