Commit bot should land patches in order of the corresponding bug's last modification date. This prevents the case where a bug is repeatedly pushed back down the commit queue by other more recently modified bugs.
Created attachment 46268 [details] Patch 1 for Bug 33395
Comment on attachment 46268 [details] Patch 1 for Bug 33395 I think this is a fantastic idea. However, this will land them in bug modification order, not patch modification order. It that what you really intended? I think the proper fix involves parsing the flag set date out of the attachment and then sorting by that in CommitQueue.next_work_item I'm about to land a patch which moves CommitQueue.next_work_item away from using fetch_patches_in_commit_queue because of bug 33638. But we can still easily sort the patches after that change. r- because this patch doesn't actually do what it claims to do.
I had figured you were trying to make it possible for users to commit-queue bugs in a certain order, but now I see you're attempting this as a performance optimization for the queue? The queue lag times will go down signifigantly as we make the tree more consistently red. The commit-queue delays are almost entirely due to flakey tests causing the tree to turn red.
When I said "consistently red" above, of course I meant "consistently green". The tree is already pretty consistently red which is the problem! :)
(In reply to comment #2) > (From update of attachment 46268 [details]) > I think this is a fantastic idea. However, this will land them in bug > modification order, not patch modification order. It that what you really > intended? I think the proper fix involves parsing the flag set date out of > the attachment and then sorting by that in CommitQueue.next_work_item Yes, I think that ideally, we'd land patches in the order in which cq+ was set on the patch. However, wasn't aware that this can be extracted from the bug. Sorting by bug last modification date seemed like a good approximation to this, as most bugs use only a single patch and the setting of the cq+ flag is usually the most recent action. So the patch does what I intended, but I agree this is not the optimal solution. > I'm about to land a patch which moves CommitQueue.next_work_item away from > using fetch_patches_in_commit_queue because of bug 33638. But we can still > easily sort the patches after that change. OK, I'll look into extracting the cq+ set date. > I had figured you were trying to make it possible for users to commit-queue > bugs in a certain order, but now I see you're attempting this as a performance > optimization for the queue? No, this wasn't intended as a performance improvement. Just to make sure that once a bug has been added to the commit queue, it doesn't get pushed down the queue by bugs added later.
Interesting the bug page with &ctype=xml does not seem to expose the date/time that the flag setter set the flag. We could parse that from https://bugs.webkit.org/show_activity.cgi?id=33395, but your initial approximation of bug-last-modify is probably close enough. It would be better to make the sort behavior explicit, but your initial change is probably sufficient.
(In reply to comment #6) > Interesting the bug page with &ctype=xml does not seem to expose the date/time > that the flag setter set the flag. We could parse that from > https://bugs.webkit.org/show_activity.cgi?id=33395, but your initial > approximation of bug-last-modify is probably close enough. It would be better > to make the sort behavior explicit, but your initial change is probably > sufficient. OK, I'll look into extracting the cq+ date from the page. Are there any other examples of parsing the buganizer page I could take a look at? In the mean time, can you r+ my original patch?
> OK, I'll look into extracting the cq+ date from the page. Are there any other > examples of parsing the buganizer page I could take a look at? There are a bunch of unit tests that call the parsing routines. You can see how they work by starting in bugzilla_unittest.py
Btw, apologies for the state of bugzilla.py. It was one of our first classes and it's in the middle of slowly being refactored.
Comment on attachment 46268 [details] Patch 1 for Bug 33395 This is better than what we do now, yes.
Landed as http://trac.webkit.org/changeset/53542 This sorts the commit queue in order of bug last modification date Leaving bug open as this fix is only an approximation to the desired behavior, as discussed.
Comment on attachment 46268 [details] Patch 1 for Bug 33395 Obsoleting patch since this was landed.
Fixed as part of bug 39519. *** This bug has been marked as a duplicate of bug 39519 ***