"Michael G. Schwern" <schw...@pobox.com> writes:

> From: "Michael G. Schwern" <schw...@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.
>
> Signed-off-by: Michael G. Schwern <schw...@pobox.com>
> ---

Looks good.

> diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
> new file mode 100644
> index 0000000..3d0bfa4
> --- /dev/null
> +++ b/perl/Git/SVN/Utils.pm
> @@ -0,0 +1,59 @@
> ...
> +=head1 FUNCTIONS
> +
> +All functions can be imported only on request.
> +
> +=head3 fatal
> +
> +    fatal(@message);
> +
> +Display a message and exit with a fatal error code.
> +
> +=cut
> +
> +# Note: not certain why this is in use instead of die.  Probably because
> +# the exit code of die is 255?  Doesn't appear to be used consistently.
> +sub fatal (@) { print STDERR "@_\n"; exit 1 }

Very true.  Also I do not think the line-noise prototype buys us
anything (other than making the code look mysterious to non Perl
programmers); we are not emulating any Perl's builtin with this
function, and I do not see a reason why we want to force list
context to its arguments, either.  But removal of it is not part of
this step anyway, so I wouldn't complain.

> +=head3 can_compress
> +
> +    my $can_compress = can_compress;
> +
> +Returns true if Compress::Zlib is available, false otherwise.
> +
> +=cut
> +
> +my $can_compress;
> +sub can_compress {
> +    return $can_compress if defined $can_compress;
> +
> +    return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;
> +}

The original said "eval { require Compress::Zlib; 1; }"; presumably,
when require does succeed, the value inside is the "1;" that has to
be at the end of Compress::Zlib, so the difference should not matter.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to