[webkit-reviews] review granted: [Bug 53569] include svn revisions in git diffs for the code review tool to use : [Attachment 80879] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 2 05:21:01 PST 2011


Adam Roben (aroben) <aroben at apple.com> has granted Ojan Vafai
<ojan at chromium.org>'s request for review:
Bug 53569: include svn revisions in git diffs for the code review tool to use
https://bugs.webkit.org/show_bug.cgi?id=53569

Attachment 80879: Patch
https://bugs.webkit.org/attachment.cgi?id=80879&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=80879&action=review

> Tools/Scripts/webkitpy/common/checkout/scm.py:758
> +	   while not revision and num_tries < 10:
> +	       # If the git checkout is not tracking an SVN repo, then
svn_revision_from_git_commit throws.
> +	       try:
> +		   revision = self.svn_revision_from_git_commit('HEAD~' +
str(num_tries))
> +	       except:
> +		   return diff
> +	       num_tries += 1

Would using "git svn info" be faster than this loop?

> Tools/Scripts/webkitpy/common/checkout/scm.py:763
> +	   return "SVN Revision: " + str(revision) + '\n' + diff

Would "Subversion" be better than "SVN"?

> Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb:22
> +	       match = /^SVN\ Revision: (.*)$/.match(line)

Would it be better to use \d instead of .?

> Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb:23
> +	       if (not match.nil?)

I don't think you need the parentheses here. You can also just use "unless"
instead of "if not".


More information about the webkit-reviews mailing list