You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/05/05 14:43:55 UTC

Re: svn commit: r941309 - /subversion/trunk/subversion/libsvn_client/commit_util.c

On Wed, May 5, 2010 at 10:27,  <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May  5 14:27:45 2010
> @@ -1036,7 +1036,7 @@ svn_client__harvest_committables(apr_has
>   for (i = 0; i < targets->nelts; ++i)
>     {
>       const svn_wc_entry_t *entry;
> -      const char *target_abspath;
> +      const char *url, *target_abspath;
>       svn_boolean_t is_added;
>       svn_error_t *err;

Please use one line per variable declaration. Most code follows this
pattern, as it is easier to read (especially when initializers are
present).

(and yes, we don't have a *rule* about this; I'm simply making a request)

>...

Cheers,
-g

Re: svn commit: r941309 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
Philip Martin wrote:
> Greg Stein <gs...@gmail.com> writes:
> 
>> On Wed, May 5, 2010 at 10:27,  <ph...@apache.org> wrote:
>>> ...
>>> +++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May  5 14:27:45 2010
>>> @@ -1036,7 +1036,7 @@ svn_client__harvest_committables(apr_has
>>>   for (i = 0; i < targets->nelts; ++i)
>>>     {
>>>       const svn_wc_entry_t *entry;
>>> -      const char *target_abspath;
>>> +      const char *url, *target_abspath;
>>>       svn_boolean_t is_added;
>>>       svn_error_t *err;
>> Please use one line per variable declaration. Most code follows this
>> pattern, as it is easier to read (especially when initializers are
>> present).
>>
>> (and yes, we don't have a *rule* about this; I'm simply making a request)
> 
> I don't see it as an improvement.  Is there a consensus that one
> declaration per line is better?

No.

I prefer to put multiple items on a line as long as none of them is
initialized, especially if they can be logically grouped:

   const char *src_path, *dst_path;
   const svn_string_t *src_props, *dst_props;
   ...

That said, my life would almost certainly go on with only negligible
inconvenience if we adopted a one-var-per-line policy.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r941309 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Greg Stein <gs...@gmail.com> writes:
> 
> > On Wed, May 5, 2010 at 10:27,  <ph...@apache.org> wrote:
> >>...
> >> +++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May  5 14:27:45 2010
> >> @@ -1036,7 +1036,7 @@ svn_client__harvest_committables(apr_has
> >>   for (i = 0; i < targets->nelts; ++i)
> >>     {
> >>       const svn_wc_entry_t *entry;
> >> -      const char *target_abspath;
> >> +      const char *url, *target_abspath;
> >>       svn_boolean_t is_added;
> >>       svn_error_t *err;
> >
> > Please use one line per variable declaration. Most code follows this
> > pattern, as it is easier to read (especially when initializers are
> > present).
> >
> > (and yes, we don't have a *rule* about this; I'm simply making a request)
> 
> I don't see it as an improvement.  Is there a consensus that one
> declaration per line is better?

I don't feel we should avoid it particularly.  Just have an eye for
what's reasonable in the context.

Quick stats from "grep":  We have nearly 900 lines of multiple
uninitialized declarations like this, out of nearly 17000 lines of
simple uninitialized declarations in total.  Half our C source files use
this style at least once, the other half don't use it.

- Julian


Re: svn commit: r941309 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> On Wed, May 5, 2010 at 10:27,  <ph...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May  5 14:27:45 2010
>> @@ -1036,7 +1036,7 @@ svn_client__harvest_committables(apr_has
>>   for (i = 0; i < targets->nelts; ++i)
>>     {
>>       const svn_wc_entry_t *entry;
>> -      const char *target_abspath;
>> +      const char *url, *target_abspath;
>>       svn_boolean_t is_added;
>>       svn_error_t *err;
>
> Please use one line per variable declaration. Most code follows this
> pattern, as it is easier to read (especially when initializers are
> present).
>
> (and yes, we don't have a *rule* about this; I'm simply making a request)

I don't see it as an improvement.  Is there a consensus that one
declaration per line is better?

-- 
Philip

Re: svn commit: r941309 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Wed, May 5, 2010 at 4:43 PM, Greg Stein <gs...@gmail.com> wrote:

> On Wed, May 5, 2010 at 10:27,  <ph...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May  5
> 14:27:45 2010
> > @@ -1036,7 +1036,7 @@ svn_client__harvest_committables(apr_has
> >   for (i = 0; i < targets->nelts; ++i)
> >     {
> >       const svn_wc_entry_t *entry;
> > -      const char *target_abspath;
> > +      const char *url, *target_abspath;
> >       svn_boolean_t is_added;
> >       svn_error_t *err;
>
> Please use one line per variable declaration. Most code follows this
> pattern, as it is easier to read (especially when initializers are
> present).
>
> (and yes, we don't have a *rule* about this; I'm simply making a request)


We probably should....

-Hyrum