Re: Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Jul 18, 2012, 00:08 UTC
- Message-ID
- <20120718000858.GH25325@burratino>
- In-Reply-To
- <5005F164.3060300@pobox.com>
Hi,
Michael G Schwern wrote:
> From 683a230e439f1d5ac2727ce4c2a74e93804fc72b Mon Sep 17 00:00:00 2001 > From: "Michael G. Schwern" <schwern@pobox.com> > Date: Wed, 11 Jul 2012 22:16:01 -0700
Just like with patch 1, the mail body should lose the above.
> Subject: [PATCH 03/11] Fix Git::SVN so it can at least compile alone.
Did I miss patch 2?
Show 6 quoted lines
> It's still very intertwined with git-svn, but that's a lot of work. This > gets things working and tests passing again (as well as they were). > > This required some parallel refactorings... > > * fatal() moved out of git-svn into a new Git::SVN::Utils
[...]
Show 12 quoted lines
> git-svn.perl | 33 ++++++++++++++++++--------------- > perl/Git/SVN.pm | 29 ++++++++++++++++++++--------- > perl/Git/SVN/Utils.pm | 19 +++++++++++++++++++ > perl/Makefile | 2 ++ > t/Git-SVN/00compile.t | 9 +++++++++ > t/Git-SVN/Utils/can_compress.t | 11 +++++++++++ > t/Git-SVN/Utils/fatal.t | 34 ++++++++++++++++++++++++++++++++++ > 7 files changed, 113 insertions(+), 24 deletions(-) > create mode 100644 perl/Git/SVN/Utils.pm > create mode 100644 t/Git-SVN/00compile.t > create mode 100644 t/Git-SVN/Utils/can_compress.t > create mode 100644 t/Git-SVN/Utils/fatal.t
It seems like a lot is going on in the one patch. Probably most of the changes are good, but if this causes a regression we would have no choice but to revert the whole thing, which would be unfeasible because of later patches building on it.
So in other words, a patch like this that makes a lot of changes at once would make life very hard for the maintainer, I imagine.
What is the motivation behind these changes? Can they be untangled from each other and applied one at a time, in such a way that each incremental change looks obviously correct?
Since I'm missing the patch that created Git/SVN.pm in the first place, I can't tell --- did that patch break the build and this one fixes it? In that case, the order of the two patches should be swapped to ensure "git bisect" is still usable.
Sorry, I wish I had better news to mix in with all this. I am thrilled to see this is making the internal APIs saner and adding tests so I hope we can get it in in a way that makes regressions unlikely.
Thanks, Jonathan