Re: Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)

[Jonathan Nieder <jrnieder@gmail.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?

> 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
[...]
>  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