Tobias Schulte <tobias.schu...@gliderpilot.de> wrote:
> This parameter is equivalent to the parameter --parents on svn cp commands
> and is useful for non-standard repository layouts.

This looks useful.  A few minor cosmetic issues.

> +++ b/Documentation/git-svn.txt
> @@ -298,6 +298,11 @@ where <name> is the name of the SVN repository as 
> specified by the -R option to
>       git config --get-all svn-remote.<name>.commiturl
>  +
>  
> +--parents;;
> +     Create parent folders. This parameter is equivalent to the parameter 
> +     --parents on svn cp commands and is useful for non-standard repository 
> +     layouts.

Trailing whitespace.

> +sub mk_parent_dirs {
> +     my $ctx = shift;
> +     my $parent = shift;

I prefer declaring multiple variables from arguments as:

        my ($ctx, $parent) = @_;

> +     $parent =~ s/\/[^\/]*$//;

You can avoid leaning toothpick syndrome (escaping '/') by specifying
alternate delimiters:

        $parent =~ s{/[^/]*$}{};

ref: perlop(1)

> +     if (!eval{$ctx->ls($parent, 'HEAD', 0)}) {
> +             mk_parent_dirs($ctx, $parent);
> +             print "Creating parent folder ${parent} ...\n";
> +             $ctx->mkdir($parent)
> +                     unless $_dry_run;

The newline is confusing, I prefer:

                $ctx->mkdir($parent) unless $_dry_run;

Howeve :

        if (!$_dry_run) {
                $ctx->mkdir($parent);
        }

May be preferred, too (especially for the non-Perl-fluent)

> +++ b/t/t9167-git-svn-cmd-branch-subproject.sh

> +test_expect_success 'initialize svnrepo' '
> +    mkdir import &&
> +    (
> +        (cd import &&
> +        mkdir -p trunk/project branches tags &&
> +        (cd trunk/project &&
> +        echo foo > foo
> +        ) &&

Tabs for all indentation, and indent consistently for subshells:

        mkdir import &&
        (
                cd import &&
                mkdir .. &&
                (
                        cd .. &&
                        ..
                )
        )

We use subshells + cd like this so it's easier to read/maintain.

Thanks again, looking forward to applying v2.
--
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