Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 27, 2012, 05:18 UTC
- Message-ID
- <7vobn13hps.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1343344945-3717-2-git-send-email-schwern@pobox.com>
"Michael G. Schwern" <schwern@pobox.com> writes:
Show 14 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. > > Signed-off-by: Michael G. Schwern <schwern@pobox.com> > ---
Looks good.
Show 22 quoted lines
> 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.
Show 14 quoted lines
> +=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.