On Oct 16, 2007, at 9:43 AM, Eric Wong wrote:
Benoit Sigoure <tsuna@xxxxxxxxxxxxx> wrote:* git-svn.perl (&traverse_ignore): Remove. (&prop_walk): New. (&cmd_show_ignore): Use prop_walk. Signed-off-by: Benoit Sigoure <tsuna@xxxxxxxxxxxxx>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 :)
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?
I'm not sure about 5/5, it's purely a style issue, however I don'treally feel strongly about a trailing "\n" either way... Nevertheless,it is definitely not part of this series and should be treated independently.
Indeed.
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.
---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
[...]
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
Attachment:
PGP.sig
Description: This is a digitally signed message part
