On 2012.7.24 3:02 PM, Jonathan Nieder wrote: >> The Git::SVN extraction is more complicated than the rest, so I'll probably do >> that separately and bust it up into a few commits. > > All of these changes are supposed to have zero functional effect, > right? Right. They're just class extraction refactoring. I don't even need to make new classes, they already exist. It's just moving them into their own file. > Could you send the first five patches that *are* supposed to have a > functional effect? I know that they will not apply cleanly to git-svn > from git "master" or on top of each other; that's fine with me. If > the approach looks right, interested people (read: probably Ben or I :)) > can make the corresponding change in the code layout from "master". > Afterwards, we can look into all that refactoring to make later > changes easier. > > What do you think? I think that would be a lot of extra work for me, create a big mess and be harder to understand. :-/ Since I'm creating new files to store the classes, the functional changes cannot be applied without the class extractions, so I'd have to rewrite them. And they will be harder to review since the main git-svn code and the class code is mixed up in one file. And they won't have unit tests, since git-svn cannot be loaded without it running. The Git::SVN extraction isn't really complicated, it just requires a handful more work than the other classes. Just a few method extractions. The rest are essentially cut, paste & fix lexicals. I'm not sure what you're going for, but you can glance through the existing changes here https://github.com/schwern/git/commits/git-svn/fix-canonical or you can do grab it as a remote and... git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical That should give you the information you need... if I understand what you need. I feel like I don't and we're talking past each other. -- Ahh email, my old friend. Do you know that revenge is a dish that is best served cold? And it is very cold on the Internet! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html