You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2005/11/04 23:52:33 UTC

[PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Hi,
  Pl. find attached the patch with the changes suggested for the test.

Regards,
Madan.

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Madan U Sreenivasan <ma...@collab.net>.
Hi All,
  Could some committer please review the aforesent patch. Thank you.

Regards,
Madan.

On Tue, 2005-11-08 at 02:33, Madan U Sreenivasan wrote:
> Hi David, Barry and dionisos,
> 
>    I have tried to fix the problems David has suggested. Have also used
> Barry's suggestions(Thx Barry, skipping the + is really cool).
> 
>    I see that dionisos has committed my older patch at r17216 and
> r17217.
> 
>    So, heres a correction to the existing codebase, that fixes the
> problems discussed earlier, makes the test more robust and uses
> dionisos's idea of using svn_wc__loggy_remove() for file removal.
> 
> HTH,
> Regards,
> Madan.
> 
> 
> On Sat, 2005-11-05 at 07:49, David James wrote:
> > On 11/4/05, Madan U Sreenivasan <ma...@collab.net> wrote:
> > > Hi,
> > >   Pl. find attached the patch with the changes suggested for the test.
> > Hi Madan,
> > 
> > Your patch generally looks good. I have a few style suggestions, but
> > these can be fixed when we commit the patch.
> > 
> > > +      if (was_schedule == svn_wc_schedule_add)
> > > +        {
> > > +          /* remove the properties file */
> > > +          const char *svn_thang;
> > svn_thang? What's that? Can you pick a better name for this variable?
> > Perhaps "properties_filename"?
> > 
> > > +          /* Working prop file. */
> > Might it be better if this comment said "remove the properties file"
> > instead? Then we can remove the first (misplaced) comment which says
> > "remove the properties file" in front of our declaration of svn_thang.
> > 
> > > [...]
> > > +  file_add_output = "A         svn-test-work/working_copies/"
> > > +  file_add_output = file_add_output + "prop_tests-16/newfile"
> > > +  propset_output = "property 'newprop' set on 'svn-test-work/"
> > > +  propset_output = propset_output + "working_copies/prop_tests-16/newfile"
> > In Python, if a string doesn't fit on one line, we usually construct
> > strings like this:
> > file_add_output = ("A         svn-test-work/working_copies/"
> >                  + "prop_tests-16/newfile")
> > 
> > Cheers,
> > 
> > David
> > 
> > --
> > David James -- http://www.cs.toronto.edu/~james
> 
> ______________________________________________________________________
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 2005-11-10 at 05:48, Garrett Rooney wrote:
> On 11/7/05, Madan U Sreenivasan <ma...@collab.net> wrote:
> 
> >    So, heres a correction to the existing codebase, that fixes the
> > problems discussed earlier, makes the test more robust and uses
> > dionisos's idea of using svn_wc__loggy_remove() for file removal.
> 
> Looks like something's wrong here...
> 
> subversion/libsvn_wc/adm_ops.c: In function `svn_wc_delete2':
> subversion/libsvn_wc/adm_ops.c:942: error: incompatible type for
> argument 3 of `svn_wc__prop_path'

oh, thanks for looking at it... let me see whats wrong... I remember
compiling and running the tests too :(...
anyways, lemme look into it.

Regards,
Madan.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 11/11/05, Madan US <ma...@collab.net> wrote:

> I see that the svn_wc__prop_path() function signature has changed on 4th
> November. I have modified the patch to accomodate the same. Thanks for
> the review, and here comes the final(hopefully!) patch.

Committed in r17334, thanks.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Madan US <ma...@collab.net>.
> On 11/7/05, Madan U Sreenivasan <ma...@collab.net> wrote:
>
>>    So, heres a correction to the existing codebase, that fixes the
>> problems discussed earlier, makes the test more robust and uses
>> dionisos's idea of using svn_wc__loggy_remove() for file removal.
>
> Looks like something's wrong here...
>
> subversion/libsvn_wc/adm_ops.c: In function `svn_wc_delete2':
> subversion/libsvn_wc/adm_ops.c:942: error: incompatible type for
> argument 3 of `svn_wc__prop_path'
I see that the svn_wc__prop_path() function signature has changed on 4th
November. I have modified the patch to accomodate the same. Thanks for
the review, and here comes the final(hopefully!) patch.

Regards,
Madan.

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 11/7/05, Madan U Sreenivasan <ma...@collab.net> wrote:

>    So, heres a correction to the existing codebase, that fixes the
> problems discussed earlier, makes the test more robust and uses
> dionisos's idea of using svn_wc__loggy_remove() for file removal.

Looks like something's wrong here...

subversion/libsvn_wc/adm_ops.c: In function `svn_wc_delete2':
subversion/libsvn_wc/adm_ops.c:942: error: incompatible type for
argument 3 of `svn_wc__prop_path'

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Madan U Sreenivasan <ma...@collab.net>.
Hi David, Barry and dionisos,

   I have tried to fix the problems David has suggested. Have also used
Barry's suggestions(Thx Barry, skipping the + is really cool).

   I see that dionisos has committed my older patch at r17216 and
r17217.

   So, heres a correction to the existing codebase, that fixes the
problems discussed earlier, makes the test more robust and uses
dionisos's idea of using svn_wc__loggy_remove() for file removal.

HTH,
Regards,
Madan.


On Sat, 2005-11-05 at 07:49, David James wrote:
> On 11/4/05, Madan U Sreenivasan <ma...@collab.net> wrote:
> > Hi,
> >   Pl. find attached the patch with the changes suggested for the test.
> Hi Madan,
> 
> Your patch generally looks good. I have a few style suggestions, but
> these can be fixed when we commit the patch.
> 
> > +      if (was_schedule == svn_wc_schedule_add)
> > +        {
> > +          /* remove the properties file */
> > +          const char *svn_thang;
> svn_thang? What's that? Can you pick a better name for this variable?
> Perhaps "properties_filename"?
> 
> > +          /* Working prop file. */
> Might it be better if this comment said "remove the properties file"
> instead? Then we can remove the first (misplaced) comment which says
> "remove the properties file" in front of our declaration of svn_thang.
> 
> > [...]
> > +  file_add_output = "A         svn-test-work/working_copies/"
> > +  file_add_output = file_add_output + "prop_tests-16/newfile"
> > +  propset_output = "property 'newprop' set on 'svn-test-work/"
> > +  propset_output = propset_output + "working_copies/prop_tests-16/newfile"
> In Python, if a string doesn't fit on one line, we usually construct
> strings like this:
> file_add_output = ("A         svn-test-work/working_copies/"
>                  + "prop_tests-16/newfile")
> 
> Cheers,
> 
> David
> 
> --
> David James -- http://www.cs.toronto.edu/~james

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Sat, 2005-11-05 at 07:49, David James wrote:
> On 11/4/05, Madan U Sreenivasan <ma...@collab.net> wrote:
> > Hi,
> >   Pl. find attached the patch with the changes suggested for the test.
> Hi Madan,
> 
> Your patch generally looks good. I have a few style suggestions, but
> these can be fixed when we commit the patch.
> 
> > +      if (was_schedule == svn_wc_schedule_add)
> > +        {
> > +          /* remove the properties file */
> > +          const char *svn_thang;
> svn_thang?
thang is just 'a thing'... I think :D


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by Barry Scott <ba...@barrys-emacs.org>.
On Nov 5, 2005, at 02:19, David James wrote:

> file_add_output = ("A         svn-test-work/working_copies/"
>                  + "prop_tests-16/newfile")

Better yet avoid the + as python concats adjacent string like C does.

s = ('line one\n'
      'line two\n' )

Barry


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #2419: 'svn rm' of schedule add leaves working props file V2

Posted by David James <ja...@gmail.com>.
On 11/4/05, Madan U Sreenivasan <ma...@collab.net> wrote:
> Hi,
>   Pl. find attached the patch with the changes suggested for the test.
Hi Madan,

Your patch generally looks good. I have a few style suggestions, but
these can be fixed when we commit the patch.

> +      if (was_schedule == svn_wc_schedule_add)
> +        {
> +          /* remove the properties file */
> +          const char *svn_thang;
svn_thang? What's that? Can you pick a better name for this variable?
Perhaps "properties_filename"?

> +          /* Working prop file. */
Might it be better if this comment said "remove the properties file"
instead? Then we can remove the first (misplaced) comment which says
"remove the properties file" in front of our declaration of svn_thang.

> [...]
> +  file_add_output = "A         svn-test-work/working_copies/"
> +  file_add_output = file_add_output + "prop_tests-16/newfile"
> +  propset_output = "property 'newprop' set on 'svn-test-work/"
> +  propset_output = propset_output + "working_copies/prop_tests-16/newfile"
In Python, if a string doesn't fit on one line, we usually construct
strings like this:
file_add_output = ("A         svn-test-work/working_copies/"
                 + "prop_tests-16/newfile")

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james