[webkit-reviews] review granted: [Bug 26734] bugzilla-tool land-and-update needs to fix reviewers in ChangeLogs : [Attachment 31962] First attempt

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 26734: bugzilla-tool land-and-update needs to fix reviewers in ChangeLogs
https://bugs.webkit.org/show_bug.cgi?id=26734

Attachment 31962: First attempt
https://bugs.webkit.org/attachment.cgi?id=31962&action=review

------- Additional Comments from David Levin <levin at chromium.org>
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