garden-o-matic should support gardening a single port and specifying how to deal with overwritten baselines
Created attachment 171014 [details] Patch
Comment on attachment 171014 [details] Patch patch isn't quite ready for review yet; the basic idea is implemented but I need to change the way I did things in the UI / JS code, and then update all of the unit tests.
Comment on attachment 171014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171014&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/config.js:172 > + // FIXME: If currentBuilder is set, we want to limit any place that iterates over > + // a list of builders to just use the currentBuilder. instead of the full list of > + // builders for the current platform. Maybe also add a FIXME to add UI to pick a builder?
(In reply to comment #3) > (From update of attachment 171014 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=171014&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/config.js:172 > > + // FIXME: If currentBuilder is set, we want to limit any place that iterates over > > + // a list of builders to just use the currentBuilder. instead of the full list of > > + // builders for the current platform. > > Maybe also add a FIXME to add UI to pick a builder? Yup, that was my intent.
Created attachment 171533 [details] rebasing
Created attachment 171564 [details] Patch
Comment on attachment 171564 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171564&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/config.js:151 > + if (config.currentBuilder) > + return builderName == config.currentBuilder; > + Lets not have this logic in this function. Lets instead have a helper function that is the sole caller of builderApplies and make builderApplies private perhaps. > Tools/Scripts/webkitpy/tool/commands/gardenomatic.py:46 > + # FIXME: This assumes that the implementation is the first part of options.platform. I'm not sure what you mean by "implementation" here. > Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:66 > + arg_string = '?' + '&'.join("%s=%s" % (key, urllib.quote(value)) for (key, value) in args.items()) Can you just use urllib.urlencode(args) or something like that here?
Comment on attachment 171564 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171564&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/config.js:151 >> + > > Lets not have this logic in this function. Lets instead have a helper function that is the sole caller of builderApplies and make builderApplies private perhaps. Sounds good. >> Tools/Scripts/webkitpy/tool/commands/gardenomatic.py:46 >> + # FIXME: This assumes that the implementation is the first part of options.platform. > > I'm not sure what you mean by "implementation" here. "implementation" is what I call the "chromium-" or "gtk-" part of the full port name, but it's not a widely used term (in fact, it may not actually show up anywhere in the code, I'm not sure). I'll rework the comment. >> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:66 >> + arg_string = '?' + '&'.join("%s=%s" % (key, urllib.quote(value)) for (key, value) in args.items()) > > Can you just use urllib.urlencode(args) or something like that here? Oh, yeah. I'll do that. I'm not that familiar w/ urllib and forgot about that.
Created attachment 171697 [details] update w/ review feedback
(In reply to comment #8) > >> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:66 > >> + arg_string = '?' + '&'.join("%s=%s" % (key, urllib.quote(value)) for (key, value) in args.items()) > > > > Can you just use urllib.urlencode(args) or something like that here? > > Oh, yeah. I'll do that. I'm not that familiar w/ urllib and forgot about that. Sigh. Turns out urllib.urlencode encodes spaces as plus signs, and the buildbots choke on that. I added a comment to note this.
Please take another look?
Committed r133060: <http://trac.webkit.org/changeset/133060>