Re: [PATCH] add 'svn:use-commit-times' property

2013-08-22 Thread Daniel Shahaf
Masaru Tsuchiyama wrote on Thu, Aug 22, 2013 at 23:20:14 +0900: > Could you comment the updated patch? As I said, I review the style but not the actual feature. Other devs are far more familiar than I am with the client layer so I prefer to let someone else do the review. Indeed Philip has now c

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-22 Thread Masaru Tsuchiyama
Philip Martin wrote: masaru tsuchiyama writes: + if (!fb->adding_file) +{ + if (fb->use_commit_times && fb->changed_date) +{ + SVN_ERR(svn_io_set_file_affected_time(fb->changed_date, +fb->local_abspath, +

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-22 Thread Philip Martin
masaru tsuchiyama writes: > + if (!fb->adding_file) > +{ > + if (fb->use_commit_times && fb->changed_date) > +{ > + SVN_ERR(svn_io_set_file_affected_time(fb->changed_date, > +fb->local_abspath, > +

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-22 Thread Masaru Tsuchiyama
Hello Daniel Could you comment the updated patch? > I prefer that someone else review the semantics, i.e., the actual > feature being added. Does anybody comment for the feature? -- Masaru Tsuchiyama masaru tsuchiyama wrote: > You still have several places where your code exceeds 8

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-13 Thread masaru tsuchiyama
> You still have several places where your code exceeds 80 characters. Fixed at attached patch. >> Encoding problem? I see a Yen symbol where a backslash should be. What do you see in the attached file? I use Google web mailer. 2013/8/8 Daniel Shahaf > Masaru Tsuchiyama wrote on Thu, Aug 0

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-08 Thread Daniel Shahaf
Masaru Tsuchiyama wrote on Thu, Aug 08, 2013 at 23:13:54 +0900: >>>" before it is modified. Makes the working copy file >>> read-only¥n" >>>" when it is not locked. Use 'svn propdel svn:needs-lock >>> PATH...'¥n" >>>" to clear.¥n" >>> + "svn:use-co

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-08 Thread Masaru Tsuchiyama
+/** The value to force the executable property to when set. + * + * @deprecated Provided for backward compatibility with the 1.4 API. + * Use @c SVN_PROP_BOOLEAN_TRUE instead. removed this. Fix line breaks, wrap to 80 columns. fixed this. Please make the comments say more than the struct

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-07 Thread Daniel Shahaf
A review mostly of the code formatting (whitespace etc). In particular I'm not reviewing the feature being added: > Index: subversion/include/svn_props.h > === > --- subversion/include/svn_props.h(revision 1509957) > +++ subversi

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-03 Thread Masaru Tsuchiyama
> Use SVN_ERR_ASSERT instead. I fixed this in the attached patch. > The apr_time_now call looks odd. Do we need to change the file's > timestamp in this case? Do you think the file's fimestamp should be preserved afer removing the property? > You also need to update the help text for 'svn p

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-01 Thread Philip Martin
Daniel Shahaf writes: > Philip Martin wrote on Wed, Jul 31, 2013 at 15:09:10 +0100: >> Perhaps this property should be inheritable? Then it could be set on a >> directory and apply to the whole tree? > > Even if yes, can't inheritability be implemented in a future patch? > i.e., maybe we should

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-01 Thread Daniel Shahaf
Could you generate patches with svn diff -x-p , please? That makes review easier. Thanks.

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-01 Thread Daniel Shahaf
Philip Martin wrote on Wed, Jul 31, 2013 at 15:09:10 +0100: > Perhaps this property should be inheritable? Then it could be set on a > directory and apply to the whole tree? Even if yes, can't inheritability be implemented in a future patch? i.e., maybe we should commit this patch and then think

Re: [PATCH] add 'svn:use-commit-times' property

2013-08-01 Thread Ben Reser
On Wed, Jul 31, 2013 at 11:19 PM, Markus Schaber wrote: > Using another property is not compatible with inheritance, as it will be > difficult to override the setting again in deeper nested directories. > > What about disabling the feature when the property contains the test "false"? +1 Inherit

Re: [PATCH] add 'svn:use-commit-times' property

2013-07-31 Thread Philip Martin
Masaru Tsuchiyama writes: > Index: subversion/include/svn_props.h > === > --- subversion/include/svn_props.h(revision 1507762) > +++ subversion/include/svn_props.h(working copy) > @@ -412,6 +412,14 @@ > */ > #define SVN_PR

Re: [PATCH] add 'svn:use-commit-times' property

2013-07-28 Thread Masaru Tsuchiyama
Hi. I attache new patch. [[[ add 'svn:use-commit-times' property. If a file has 'svn:use-commit-times' property, the timestamp of the file is modified to commit time. * subversion/include/svn_props.h (): add SVN_PROP_USE_COMMIT_TIMES and SVN_PROP_USE_COMMIT_TIMES_VALUE (): ad

Re: [PATCH] add 'svn:use-commit-times' property

2013-07-26 Thread Philip Martin
Masaru Tsuchiyama writes: > Index: subversion/include/svn_props.h > === > --- subversion/include/svn_props.h(revision 1505676) > +++ subversion/include/svn_props.h(working copy) > @@ -449,6 +449,9 @@ > /** Property used to r

Re: [PATCH] add 'svn:use-commit-times' property

2013-07-26 Thread Masaru Tsuchiyama
Hi. Does anybody comment my patch? -- Masaru Tsuchiyama