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

On 11/4/05, Madan U Sreenivasan <madan@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