"Michael G. Schwern" <[email protected]> writes:
> From: "Michael G. Schwern" <[email protected]>
>
> This means it should be able to load without git-svn being loaded.
>
> * Load Git.pm on its own and all the needed command functions.
>
> * It needs to grab at a git-svn lexical $_prefix representing the --prefix
> option. Provide opt_prefix() for that. This is a refactoring artifact.
> The prefix should really be passed into Git::SVN->new.
I agree that the prefix is part of SVN->new arguments in the final
state after applying the whole series (not just these four but also
with the follow-up patches).
> * Unqualify unnecessarily fully qualified globals like
> $Git::SVN::default_repo_id.
>
> * Lexically isolate the class just to make sure nothing is leaking out.
> ---
Forgot to sign-off, or are you still unsure about this step?
> diff --git a/git-svn.perl b/git-svn.perl
> index 79fe4a4..9cdf6fc 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -109,6 +109,10 @@ my ($_stdin, $_help, $_edit,
> $_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,
> $_prefix, $_no_checkout, $_url, $_verbose,
> $_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
> +
> +# This is a refactoring artifact so Git::SVN can get at this git-svn switch.
> +sub opt_prefix { return $_prefix || '' }
> +
> $Git::SVN::_follow_parent = 1;
> $Git::SVN::Fetcher::_placeholder_filename = ".gitignore";
> $_q ||= 0;
> @@ -4280,12 +4292,13 @@ sub find_rev_after {
> sub _new {
> my ($class, $repo_id, $ref_id, $path) = @_;
> unless (defined $repo_id && length $repo_id) {
> - $repo_id = $Git::SVN::default_repo_id;
> + $repo_id = $default_repo_id;
> }
> unless (defined $ref_id && length $ref_id) {
> - $_prefix = '' unless defined($_prefix);
> + # Access the prefix option from the git-svn main program if
> it's loaded.
> + my $prefix = defined &::opt_prefix ? ::opt_prefix() : "";
Again, I agree with you that passing $prefix as one of the arguments
to ->new is the right thing to do in the final state after applying
the whole series. I don't know if later steps in your patch series
will do so, but it _might_ make more sense to update ->new and its
callers to do so without doing anything else first, so that you do
not have to call out to the ::opt_prefix() when you split things
out.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
