bugzilla-tool land-and-update needs to fix reviewers in ChangeLogs Right now it just tries to commit and update the bug, but doesn't fix up the ChangeLogs to have a reviewer. This command is meant for when you're landing your own patches which have been reviewed and are already in your tree (possibly with reviewer-suggested modifications). It would be easy to grab the reviewer off the patch on the bug (assuming there is only one patch). We can error-out if there is more than one patch I guess.
Ok. I've changed "land-and-update" to the shorter "land-diff". I've also made land-diff know how to build and test before actually committing. I've also made land-diff's bug_id parameter optional (since sometimes you want to land changes which don't have a bug, but you do want to do want the automated goodness of reviewer setting and one more final build and test before landing.)
Created attachment 31962 [details] First attempt
Comment on attachment 31962 [details] First attempt A few minor things to fix. Free to fix up and submit (unless you do larger changes and want another review of it). > diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool > @@ -86,6 +87,11 @@ def latest_changelog_entry(changelog_path): > +def set_reviewer_in_changelog(changelog_path, reviewer): > + # inplace=1 creates a backup file and re-directs stdout to the file > + for line in fileinput.FileInput(changelog_path, inplace=1): > + print line.replace("NOBODY (OOPS!)", reviewer), Two comments: 1. Indent off by one space ("print line....") 2. Why is there a comma at the end of this line? (Can it be removed?) > class LandAndUpdateBug(Command): > def execute(self, options, args, tool): > + self.update_changelogs_with_reviewer(options.reviewer, bug_id, tool) > + It seems like it should update the date in the changelog as well. Could be a fix me for now. > + else: > + log(comment_text) If the comment contains characters like \n, then I think it will mess up the output. Maybe the log function should be updated to look like this def log(string): print >> sys.stderr, "%s" % string
(In reply to comment #3) > (From update of attachment 31962 [details] [review]) > A few minor things to fix. Free to fix up and submit (unless you do larger > changes and want another review of it). Thanks! > > diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool > > @@ -86,6 +87,11 @@ def latest_changelog_entry(changelog_path): > > +def set_reviewer_in_changelog(changelog_path, reviewer): > > + # inplace=1 creates a backup file and re-directs stdout to the file > > + for line in fileinput.FileInput(changelog_path, inplace=1): > > + print line.replace("NOBODY (OOPS!)", reviewer), > > Two comments: > 1. Indent off by one space ("print line....") > 2. Why is there a comma at the end of this line? (Can it be removed?) 1. Fixed 2. To make print not add a newline when printing: http://www.python.org/doc/2.4.4/ref/print.html A "\n" character is written at the end, unless the print statement ends with a comma. This is the only action if the statement contains just the keyword print. > > class LandAndUpdateBug(Command): > > def execute(self, options, args, tool): > > + self.update_changelogs_with_reviewer(options.reviewer, bug_id, tool) > > + > > It seems like it should update the date in the changelog as well. Could be a > fix me for now. Yeah. Will add a FIXME for now. > > + else: > > + log(comment_text) > > If the comment contains characters like \n, then I think it will mess up the > output. Maybe the log function should be updated to look like this > > def log(string): > print >> sys.stderr, "%s" % string log("foo\nbar") seems to work just fine? Thanks for the review!
Git got upset with itself. But this landed as r45351.