Re: [PATCH 1/5] Add a generic tree traversal to fetch SVN properties.
- From
Benoit SIGOURE <tsuna@lrde.epita.fr>
- Date
- Oct 16, 2007, 09:35 UTC
- Message-ID
- <07B09A42-73DD-4565-975F-A4E40BF862AC@lrde.epita.fr>
- In-Reply-To
- <20071016074310.GA32254@soma>
On Oct 16, 2007, at 9:43 AM, Eric Wong wrote:
Show 9 quoted lines
> Benoit Sigoure <tsuna@lrde.epita.fr> wrote: >> * git-svn.perl (&traverse_ignore): Remove. >> (&prop_walk): New. >> (&cmd_show_ignore): Use prop_walk. >> >> Signed-off-by: Benoit Sigoure <tsuna@lrde.epita.fr> > > Although I myself have never needed this functionality, this series > looks pretty good in general.
I heavily script Git with my own wrappers and having this sort if functionality does enhance the scriptability of git-svn.
> > Thanks.
You're welcome :)
Show 12 quoted lines
> > One comment below about property selection (whitelist vs blacklist). > > > It would be possible to get identical information out of > unhandled.log, > but older repositories may not have complete information... Maybe > some > local option would be good for people with complete unhandled.log > files; > but it could be really incomplete/insufficient. >
In order to avoid using SVN::Ra and avoid access to the SVN repo? Hmm, clever, I didn't think about this. Maybe we can provide both, the default would check unhandled.log and an option would enable direct access to the SVN repo?
Show 7 quoted lines
> > I'm not sure about 5/5, it's purely a style issue, however I don't > really feel strongly about a trailing "\n" either way... > Nevertheless, > it is definitely not part of this series and should be treated > independently. >
Indeed.
Show 10 quoted lines
>
> Coding style
>
> Other than that, I prefer to keep braces on the same line as foreach,
> if, else statements. I generally follow the git and Linux coding
> style for C in my Perl code.
>
> One exception that I make for Perl (but not C) is that I keep the "{"
> for subs on the same line (since subs can be nested and anonymous ones
> passed as arguments and such); unlike their C counterparts[1]Indeed, sorry, I started correctly but then completely forgot to follow the existing Coding Style. The CS I use daily is totally different, sorry ;) Shall I resend the patch series with corrected CS?
> > [1] - well, nesting functions is allowed in C99 or GNU C, I can't > remember which or both... >
GNU C, AFAIR.
Show 9 quoted lines
>> --- >> git-svn.perl | 66 +++++++++++++++++++++++++++++++++++++++ >> +----------------- >> 1 files changed, 46 insertions(+), 20 deletions(-) >> >> diff --git a/git-svn.perl b/git-svn.perl >> index 777e436..abc83ec 100755 >> --- a/git-svn.perl >> +++ b/git-svn.perl
[...]
Show 19 quoted lines
>
> How about having a blacklist (for the author, date, log, uuid?)
> instead
> of a whitelist? I can't remember all of them that should be
> blacklisted,
> but maybe it's just author, date and log)..
>
>> + my $interesting_props = 0;
>> + foreach(keys %{$props})
>> + {
>> + # If it doesn't start with `svn:', it must be a
>> + # user-defined property.
>> + ++$interesting_props and next if $_ !~ /^svn:/;
>> + # FIXME: Fragile, if SVN adds new public properties,
>> + # this needs to be updated.
>> + ++$interesting_props if /^svn:(?:ignore|keywords|executable
>> + |eol-style|mime-type
>> + |externals|needs-lock)$/x;
>> + }Why not. I thought that the SVN internals were more subject to change than the public "interface", hence the check.
>> + &$sub($self, $p, $props) if $interesting_props; >> +
PS: For some reason, the introduction message didn't make its way to the ML. I made a mistake when sending it because I first ran git send-email --compose, then noticed that it sent only one mail, and ran git send-email *.patch afterwards. Weird.
Cheers,
-- Benoit Sigoure aka Tsuna EPITA Research and Development Laboratory