On 15.05.2013 04:35, Eric Wong wrote:
>> + 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;
In fact, this was a copy/paste from a few lines above
print "Copying ${src} at r${rev} to ${dst}...\n";
$ctx->copy($src, $rev, $dst)
unless $_dry_run;
> Howeve :
>
> if (!$_dry_run) {
> $ctx->mkdir($parent);
> }
>
> May be preferred, too (especially for the non-Perl-fluent)
I am a non-Perl-fluent, as I come from the Java world with some
knowledge of C and C++. But to be able to create the patch I had to
gather some Perl knowledge, and by doing this, I learned enough to
understand, that there is more than one way, ... Especially the
constructs if (condition) foo(); vs foo() if (condition); and the same
for unless. And since the dry_run is the exception in this case, I think
unless is a valid choice -- and is used often in git-svn.perl. So I will
stick to it, but remove the newline.
>
>> +++ 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.
Again, this was a copy/paste from t9128-git-svn-cmd-branch.sh. So this
file needs some cosmetics, too. And t9127... as well...
>
> Thanks again, looking forward to applying v2.
>
I will send v2 soon.
Tobias
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html