You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexey Neyman <st...@att.net> on 2010/08/11 01:22:53 UTC
[PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Okay, try again:
[[[
Fix the type of structures returned in bindings from svn_fs_paths_changed2().
* subversion/include/svn_fs.h
(svn_fs_paths_changed2): Rename the argument from changed_paths_p to
changed_paths_p2, so that it's different from argument to
svn_fs_paths_changed().
* subversion/bindings/swig/svn_fs.i
(changed_paths_p2): New %hash_argout_typemap, denotes apr_hash_t containing
svn_fs_path_change2_t.
]]]
Regards,
Alexey.
On Tuesday, August 10, 2010 06:04:14 pm Hyrum K. Wright wrote:
> When submitting patches, it is helpful to provide a log message. (See
> http://subversion.apache.org/docs/community-guide/general.html#log-messages
> )
>
> Thanks,
> -Hyrum
>
> On Tue, Aug 10, 2010 at 7:38 PM, Alexey Neyman <st...@att.net> wrote:
> > Small update to the patch: change doxygen comment to match new argument
> > name.
> >
> > Regards,
> > Alexey.
> >
> > On Tuesday, August 10, 2010 02:53:50 pm Alexey Neyman wrote:
> >> Hi all,
> >>
> >> Looks like the binding for svn_fs_paths_changed2() incorrectly specifies
> >> the type of structures contained in the hash it returns: the following
> >> code
> >>
> >> s = fs.paths_changed2(rev_root, pool)
> >> for i in s:
> >> sys.stderr.write("%s = %s\n" % (i, repr(s)))
> >>
> >> indicates that bindings assume the hash to contain svn_fs_path_change_t
> >> structures, not svn_fs_path_change2_t as it should (it's the difference
> >> between fs.paths_changed() and fs.paths_changed2(), actually).
> >>
> >> The attached patch fixes this issue.
> >>
> >> Regards,
> >> Alexey.
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by Alexey Neyman <st...@att.net>.
There was a race condition :) C. Michael Pilato committed the same patch at
the same time as I sent updated version, with less than 3 minutes difference.
Regards,
Alexey.
On Wednesday, August 18, 2010 03:54:43 pm Gavin Beau Baumanis wrote:
> Ping. The renewed patch submission has received no comments.
>
>
> Gavin.
>
> On 12/08/2010, at 6:12 AM, Alexey Neyman wrote:
> > On Wednesday, August 11, 2010 12:10:41 pm C. Michael Pilato wrote:
> >> On 08/10/2010 09:22 PM, Alexey Neyman wrote:
> >>> Okay, try again:
> >>>
> >>> [[[
> >>> Fix the type of structures returned in bindings from
> >>> svn_fs_paths_changed2().
> >>>
> >>> * subversion/include/svn_fs.h
> >>>
> >>> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
> >>> changed_paths_p2, so that it's different from argument to
> >>> svn_fs_paths_changed().
> >>
> >> Minor nit -- I think 'changed_paths2_p' would be a better name. To me,
> >> a number at the very end of a parameter name says "differentiator" (as
> >> in 'strcmp(str1, str2)') whereas having that '2' closer to the
> >> 'changed_paths' name says "version iterator". But like I said, it's a
> >> minor nit.
> >>
> >> Otherwise, +1 on the patch.
> >
> > Agreed. Updated patch attached.
> >
> > Regards,
> > Alexey.
> > <fs_paths_changed2.diff>
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by Gavin Beau Baumanis <ga...@thespidernet.com>.
Ping. The renewed patch submission has received no comments.
Gavin.
On 12/08/2010, at 6:12 AM, Alexey Neyman wrote:
> On Wednesday, August 11, 2010 12:10:41 pm C. Michael Pilato wrote:
>> On 08/10/2010 09:22 PM, Alexey Neyman wrote:
>>> Okay, try again:
>>>
>>> [[[
>>> Fix the type of structures returned in bindings from
>>> svn_fs_paths_changed2().
>>>
>>> * subversion/include/svn_fs.h
>>>
>>> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
>>> changed_paths_p2, so that it's different from argument to
>>> svn_fs_paths_changed().
>>
>> Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
>> number at the very end of a parameter name says "differentiator" (as in
>> 'strcmp(str1, str2)') whereas having that '2' closer to the 'changed_paths'
>> name says "version iterator". But like I said, it's a minor nit.
>>
>> Otherwise, +1 on the patch.
>
> Agreed. Updated patch attached.
>
> Regards,
> Alexey.
> <fs_paths_changed2.diff>
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by Alexey Neyman <st...@att.net>.
On Wednesday, August 11, 2010 12:10:41 pm C. Michael Pilato wrote:
> On 08/10/2010 09:22 PM, Alexey Neyman wrote:
> > Okay, try again:
> >
> > [[[
> > Fix the type of structures returned in bindings from
> > svn_fs_paths_changed2().
> >
> > * subversion/include/svn_fs.h
> >
> > (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
> > changed_paths_p2, so that it's different from argument to
> > svn_fs_paths_changed().
>
> Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
> number at the very end of a parameter name says "differentiator" (as in
> 'strcmp(str1, str2)') whereas having that '2' closer to the 'changed_paths'
> name says "version iterator". But like I said, it's a minor nit.
>
> Otherwise, +1 on the patch.
Agreed. Updated patch attached.
Regards,
Alexey.
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/15/2010 03:05 PM, Alexey Neyman wrote:
> Hi all,
>
> On Wednesday, August 11, 2010 01:09:50 pm C. Michael Pilato wrote:
>> On 08/11/2010 03:10 PM, C. Michael Pilato wrote:
>>> On 08/10/2010 09:22 PM, Alexey Neyman wrote:
>>>> Okay, try again:
>>>>
>>>> [[[
>>>> Fix the type of structures returned in bindings from
>>>> svn_fs_paths_changed2().
>>>>
>>>> * subversion/include/svn_fs.h
>>>>
>>>> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
>>>> changed_paths_p2, so that it's different from argument to
>>>> svn_fs_paths_changed().
>>>
>>> Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
>>> number at the very end of a parameter name says "differentiator" (as in
>>> 'strcmp(str1, str2)') whereas having that '2' closer to the
>>> 'changed_paths' name says "version iterator". But like I said, it's a
>>> minor nit.
>>>
>>> Otherwise, +1 on the patch.
>>
>> Committed with the aforementioned tweaks in r984565. Thanks, Alexey.
>
> Could this patch be picked up in 1.6.14?
>
> Regards,
> Alexey.
I've made the proposal. Now, we just need votes.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by Alexey Neyman <st...@att.net>.
Hi all,
On Wednesday, August 11, 2010 01:09:50 pm C. Michael Pilato wrote:
> On 08/11/2010 03:10 PM, C. Michael Pilato wrote:
> > On 08/10/2010 09:22 PM, Alexey Neyman wrote:
> >> Okay, try again:
> >>
> >> [[[
> >> Fix the type of structures returned in bindings from
> >> svn_fs_paths_changed2().
> >>
> >> * subversion/include/svn_fs.h
> >>
> >> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
> >> changed_paths_p2, so that it's different from argument to
> >> svn_fs_paths_changed().
> >
> > Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
> > number at the very end of a parameter name says "differentiator" (as in
> > 'strcmp(str1, str2)') whereas having that '2' closer to the
> > 'changed_paths' name says "version iterator". But like I said, it's a
> > minor nit.
> >
> > Otherwise, +1 on the patch.
>
> Committed with the aforementioned tweaks in r984565. Thanks, Alexey.
Could this patch be picked up in 1.6.14?
Regards,
Alexey.
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/11/2010 03:10 PM, C. Michael Pilato wrote:
> On 08/10/2010 09:22 PM, Alexey Neyman wrote:
>> Okay, try again:
>>
>> [[[
>> Fix the type of structures returned in bindings from svn_fs_paths_changed2().
>>
>> * subversion/include/svn_fs.h
>> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
>> changed_paths_p2, so that it's different from argument to
>> svn_fs_paths_changed().
>
> Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
> number at the very end of a parameter name says "differentiator" (as in
> 'strcmp(str1, str2)') whereas having that '2' closer to the 'changed_paths'
> name says "version iterator". But like I said, it's a minor nit.
>
> Otherwise, +1 on the patch.
Committed with the aforementioned tweaks in r984565. Thanks, Alexey.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/10/2010 09:22 PM, Alexey Neyman wrote:
> Okay, try again:
>
> [[[
> Fix the type of structures returned in bindings from svn_fs_paths_changed2().
>
> * subversion/include/svn_fs.h
> (svn_fs_paths_changed2): Rename the argument from changed_paths_p to
> changed_paths_p2, so that it's different from argument to
> svn_fs_paths_changed().
Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a
number at the very end of a parameter name says "differentiator" (as in
'strcmp(str1, str2)') whereas having that '2' closer to the 'changed_paths'
name says "version iterator". But like I said, it's a minor nit.
Otherwise, +1 on the patch.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand