[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