Bug 13732

Summary: prepare-ChangeLog should work with git
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, ddkilzer
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Other   
OS: OS X 10.4   
Attachments:
Description Flags
patch to make prepare-ChangeLog work with git-svn
none
More complete git support for prepare-ChangeLog ddkilzer: review+

Simon Hausmann
Reported 2007-05-15 09:19:46 PDT
When using git-svn to check out WebKit the prepare-ChangeLog script does not work anymore. The attached patch tries to distinguish the presence of a svn checkout or a git repository and calls git diff in the latter case.
Attachments
patch to make prepare-ChangeLog work with git-svn (5.85 KB, patch)
2007-05-15 09:24 PDT, Simon Hausmann 		no flags
DetailsFormatted DiffDiff
More complete git support for prepare-ChangeLog (11.82 KB, patch)
2007-05-15 10:07 PDT, Adam Roben (:aroben) 		ddkilzer: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Hausmann
Comment 1 2007-05-15 09:24:13 PDT
Created attachment 14567 [details] patch to make prepare-ChangeLog work with git-svn
Adam Roben (:aroben)
Comment 2 2007-05-15 09:54:13 PDT
Comment on attachment 14567 [details] patch to make prepare-ChangeLog work with git-svn I've been working on this as well. I think this patch will have problems when running prepare-ChangeLog from within a subdirectory. I'm going to upload my patch that does what this one does and more.
Adam Roben (:aroben)
Comment 3 2007-05-15 10:07:42 PDT
Created attachment 14568 [details] More complete git support for prepare-ChangeLog
Simon Hausmann
Comment 4 2007-05-15 12:46:28 PDT
Great, your patch does indeed more and looks a lot cleaner. I'm all for it, anything that makes working with git and webkit easier :)
David Kilzer (:ddkilzer)
Comment 5 2007-05-15 16:42:11 PDT
Comment on attachment 14568 [details] More complete git support for prepare-ChangeLog Overall this patch looks great! I assume you've been living on it for a while, Adam? A couple comments: - Shouldn't %svn in isModifiedOrAddedStatus() also contain "R" for replaced files? - Using diffHeaderFormat() within loops (when it returns the same value each time) and redefining local data structures for each method call (like in isModifiedOrAddedStatus() and isConflictStatus() and statusDescription()) makes me cringe, but unless there is an apparent slow down, it's nice not to have to declare global variables that are farther away from the subroutines. - I understand the reason for declaring $isSVN, $isGit and $gitRoot next to their methods, but I'm torn about not having them at the top of the script, too. They're okay, though. r=me if you address "R" in %svn in isModifiedOrAddedStatus().
Adam Roben (:aroben)
Comment 6 2007-05-15 16:57:01 PDT
(In reply to comment #5) > (From update of attachment 14568 [details] [edit]) > Overall this patch looks great! I assume you've been living on it for a while, > Adam? Yep, and it seems to be working nicely, modulo a few FIXMEs in the script. > A couple comments: > > - Shouldn't %svn in isModifiedOrAddedStatus() also contain "R" for replaced > files? Yes. Fixed. > - Using diffHeaderFormat() within loops (when it returns the same value each > time) and redefining local data structures for each method call (like in > isModifiedOrAddedStatus() and isConflictStatus() and statusDescription()) makes > me cringe, but unless there is an apparent slow down, it's nice not to have to > declare global variables that are farther away from the subroutines. I'd guess that, as you said, these won't slow things down much. Thanks for the review!
Adam Roben (:aroben)
Comment 7 2007-05-15 16:59:40 PDT
Committed r21501
David Kilzer (:ddkilzer)
Comment 8 2007-05-27 19:19:13 PDT
*** Bug 13884 has been marked as a duplicate of this bug. ***
Simon Hausmann
Comment 9 2007-05-28 02:24:52 PDT
Why is 13884 a duplicate of this bug? This bug report was about the initial git support for prepare-ChangeLog, bug 13884 is about extending it to operate on a given git commit instead of just changes in the working directory.
Note You need to log in before you can comment on or make changes to this bug.