Re: [PATCH] git-svn: introduce --parents parameter for commands branch and tag
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: introduce --parents parameter for commands branch and tag
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