33395 2010-01-08 12:06:23 -0800 Commit bot should land patches in order of last modification date 2010-05-21 17:11:21 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED DUPLICATE 39519 P2 Normal --- 1 steveblock steveblock abarth eric steveblock oldest_to_newest 178739 0 steveblock 2010-01-08 12:06:23 -0800 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. 179326 1 46268 steveblock 2010-01-11 04:09:59 -0800 Created attachment 46268 Patch 1 for Bug 33395 180905 2 46268 eric 2010-01-14 13:56:13 -0800 Comment on attachment 46268 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. 180906 3 eric 2010-01-14 13:57:32 -0800 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. 180909 4 eric 2010-01-14 14:02:49 -0800 When I said "consistently red" above, of course I meant "consistently green". The tree is already pretty consistently red which is the problem! :) 181167 5 steveblock 2010-01-15 02:29:55 -0800 (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. 181263 6 eric 2010-01-15 10:06:51 -0800 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. 182019 7 steveblock 2010-01-18 15:35:56 -0800 (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? 182028 8 abarth 2010-01-18 16:09:50 -0800 > 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 182029 9 abarth 2010-01-18 16:10:36 -0800 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. 182724 10 46268 eric 2010-01-20 03:45:35 -0800 Comment on attachment 46268 Patch 1 for Bug 33395 This is better than what we do now, yes. 182732 11 steveblock 2010-01-20 03:59:17 -0800 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. 187155 12 46268 eric 2010-02-02 14:31:06 -0800 Comment on attachment 46268 Patch 1 for Bug 33395 Obsoleting patch since this was landed. 229183 13 eric 2010-05-21 17:11:21 -0700 Fixed as part of bug 39519. *** This bug has been marked as a duplicate of bug 39519 *** 46268 2010-01-11 04:09:59 -0800 2010-02-02 14:31:06 -0800 Patch 1 for Bug 33395 commitBotOrder.txt text/plain 2066 steveblock SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUzMDY5KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTAtMDEtMTEgIFN0ZXZlIEJsb2NrICA8c3Rl dmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEZpeCBjb21taXQgYm90IHRvIGxhbmQgcGF0Y2hlcyBpbiBvcmRlciBvZiBs YXN0IG1vZGlmaWNhdGlvbiBkYXRlLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MzMzOTUKKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvYnVnemls bGEucHk6IE1vZGlmaWVkLiBBZGRlZCAnb3JkZXI9TGFzdCtDaGFuZ2VkJyB0byBidWd6aWxsYSBj b21taXQgcXVldWUgVVJMLgorCiAyMDEwLTAxLTEwICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtp dC5vcmc+CiAKICAgICAgICAgUnViYmVyIHN0YW1wZWQgYnkgRGF2aWQgS2lsemVyLgpJbmRleDog V2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9idWd6aWxsYS5weQo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBX ZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L2J1Z3ppbGxhLnB5CShyZXZpc2lvbiA1Mjk4NykK KysrIFdlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvYnVnemlsbGEucHkJKHdvcmtpbmcgY29w eSkKQEAgLTgxLDcgKzgxLDcgQEAgY2xhc3MgQnVnKG9iamVjdCk6CiAgICAgICAgIHJldHVybiBb cGF0Y2ggZm9yIHBhdGNoIGluIHNlbGYucGF0Y2hlcygpIGlmIHBhdGNoLmdldCgicmV2aWV3Iikg PT0gIj8iXQogCiAKLSMgQSBjb250YWluZXIgZm9yIGFsbCBvZiB0aGUgbG9naWMgZm9yIG1ha2lu ZyBhIHBhcnNpbmcgYnV6aWxsYSBxdWVyaWVzLgorIyBBIGNvbnRhaW5lciBmb3IgYWxsIG9mIHRo ZSBsb2dpYyBmb3IgbWFraW5nIGFuZCBwYXJzaW5nIGJ1emlsbGEgcXVlcmllcy4KIGNsYXNzIEJ1 Z3ppbGxhUXVlcmllcyhvYmplY3QpOgogICAgIGRlZiBfX2luaXRfXyhzZWxmLCBidWd6aWxsYSk6 CiAgICAgICAgIHNlbGYuYnVnemlsbGEgPSBidWd6aWxsYQpAQCAtMTE0LDcgKzExNCw3IEBAIGNs YXNzIEJ1Z3ppbGxhUXVlcmllcyhvYmplY3QpOgogICAgICAgICByZXR1cm4gc3VtKFtzZWxmLmJ1 Z3ppbGxhLmZldGNoX3Jldmlld2VkX3BhdGNoZXNfZnJvbV9idWcoYnVnX2lkKSBmb3IgYnVnX2lk IGluIHNlbGYuZmV0Y2hfYnVnX2lkc19mcm9tX3BlbmRpbmdfY29tbWl0X2xpc3QoKV0sIFtdKQog CiAgICAgZGVmIGZldGNoX2J1Z19pZHNfZnJvbV9jb21taXRfcXVldWUoc2VsZik6Ci0gICAgICAg IGNvbW1pdF9xdWV1ZV91cmwgPSAiYnVnbGlzdC5jZ2k/cXVlcnlfZm9ybWF0PWFkdmFuY2VkJmJ1 Z19zdGF0dXM9VU5DT05GSVJNRUQmYnVnX3N0YXR1cz1ORVcmYnVnX3N0YXR1cz1BU1NJR05FRCZi dWdfc3RhdHVzPVJFT1BFTkVEJmZpZWxkMC0wLTA9ZmxhZ3R5cGVzLm5hbWUmdHlwZTAtMC0wPWVx dWFscyZ2YWx1ZTAtMC0wPWNvbW1pdC1xdWV1ZSUyQiIKKyAgICAgICAgY29tbWl0X3F1ZXVlX3Vy bCA9ICJidWdsaXN0LmNnaT9xdWVyeV9mb3JtYXQ9YWR2YW5jZWQmYnVnX3N0YXR1cz1VTkNPTkZJ Uk1FRCZidWdfc3RhdHVzPU5FVyZidWdfc3RhdHVzPUFTU0lHTkVEJmJ1Z19zdGF0dXM9UkVPUEVO RUQmZmllbGQwLTAtMD1mbGFndHlwZXMubmFtZSZ0eXBlMC0wLTA9ZXF1YWxzJnZhbHVlMC0wLTA9 Y29tbWl0LXF1ZXVlJTJCJm9yZGVyPUxhc3QrQ2hhbmdlZCIKICAgICAgICAgcmV0dXJuIHNlbGYu X2ZldGNoX2J1Z19pZHNfYWR2YW5jZWRfcXVlcnkoY29tbWl0X3F1ZXVlX3VybCkKIAogICAgIGRl ZiBmZXRjaF9wYXRjaGVzX2Zyb21fY29tbWl0X3F1ZXVlKHNlbGYsIHJlamVjdF9pbnZhbGlkX3Bh dGNoZXM9RmFsc2UpOgo=