EWS django app should resubmit patches to commit-queue which were cq- by commit-queue (e.g.: due to flaky test failure) and later were cq+. Currently ews django app does not re-submit any patch, it sends the patch to buildbot only once. Because of this is a patch which was rejected by commit-queue, and later cq+ again by patch author, would not be picked up again.
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].
<rdar://problem/60561892>