Masaru Tsuchiyama wrote on Thu, Aug 08, 2013 at 22:17:05 +0900:
> > > + cmd = [ self.__svnadmin_path, "dump",
> > > + "--incremental", "-r", revparam,
> self.__repospath ]
> >
> > That doesn't look right. If self.__repospath can be a local path to a
> > repository root, you shouldn't pass it as an argument to 'svn' a few
> lines
> > above.
>
> If self.__repospath is a local path to a repository,
> I don't pass it to svn. See get_head_rev().
>
In the patch I reviewed, there was a
+ cmd = [ self.__svn_path, "youngest", self.__repospath ]
call a few lines above. I left it in the quoted context of my review.
I guess you mean you fixed that issue in the new iteration of the patch.
>> Also, it'd be good practice to pass "--" in front of self.__repospath, but
>> that
>> appears to be a preexisting problem in the code (i.e., not introduced by your
>> patch).
>
> What is the purpose in passing "--"?
>
Indicating that further argv arguments are paths or URLs and not
options, even if they begin with '-'. (Note that we set
os->interleave=1 in sub_main().)
> > ... and while at it, use r"" literals to avoid clashes with a
> potential future
> > "r\d" backslash escape sequence.
>
> Do you worry about changing format change of 'svn log'
> in later versions of svn?
>
No, I worry about the Python language defining a meaning to the escape
sequence
\d
inside a double-quoted string, just like it already defines
\n
to be a newline character (and not literal backslash followed by literal
small letter N).
>
Thanks for the fixes.
Daniel
