Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.

From: Eric Wong <normalperson@yhbt.net>
Date: Jul 25, 2012, 21:24 UTC
Message-ID: <20120725212418.GA17494@dcvr.yhbt.net>
In-Reply-To: <1343196066-81319-2-git-send-email-schwern@pobox.com>

"Michael G. Schwern" <schwern@pobox.com> wrote:

Show 11 quoted lines

> From: "Michael G. Schwern" <schwern@pobox.com>
> 
> Put them in a new module called Git::SVN::Utils.  Yeah, not terribly
> original and it will be a dumping ground.  But its better than having
> them in the main git-svn program.  At least they can be documented
> and tested.
> 
> * fatal() is used by many classes.
> * Change the $can_compress lexical into a function.
> 
> This should be enough to extract Git::SVN.

Please keep Jonathan Cc:-ed, he's been very helpful with this series (and very helpful in general :)

This series is mostly alright by me, a few minor comments inline.

> --- /dev/null
> +++ b/t/Git-SVN/00compile.t
> +
> +use Test::More tests => 1;

> +++ b/t/Git-SVN/Utils/fatal.t
> @@ -0,0 +1,34 @@
> +
> +use Test::More 'no_plan';

Didn't we agree to use done_testing()? Perhaps (as you suggested) with a private copy of Test::More? It's probably easier to start using done_testing() earlier rather than later.

Show 7 quoted lines

> +BEGIN {
> +    # Override exit at BEGIN time before Git::SVN::Utils is loaded
> +    # so it will see our local exit later.
> +    *CORE::GLOBAL::exit = sub(;$) {
> +        return @_ ? CORE::exit($_[0]) : CORE::exit();
> +    };
> +}

For new code related to git-svn, please match the existing indentation style (tabs) prevalent in git-svn. Most of the Perl found in git also uses tabs for indentation.