Re: svn commit: r921848 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c

On Mar 11, 2010, at 9:11 PM, Neels J Hofmeyr wrote:

> Greg Stein wrote:
>> On Thu, Mar 11, 2010 at 08:52, <neels_at_apache.org> wrote:
>>> ...
>>> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Thu Mar 11 13:52:15 2010
>>> ...
>>> @@ -2219,9 +2229,32 @@ svn_wc__get_pristine_contents(svn_stream
>>> return SVN_NO_ERROR;
>>> }
>>> }
>>> + else
>>> + if (status == svn_wc__db_status_base_deleted)
>>
>> Woah. This formatting is incorrect. We always do "else if (...". The
>> code above almost makes it look like the "if" is totally separate
>> since it is at the same indentation level, but it ISN'T. The else
>> dramatically changes the meaning.
>>
>> The above style is used nowhere else in our code. Please fix the
>> several uses in this function.
>
> Hm, I've used this before, always have.
> IMHO it looks much better this way, and is easier to edit around...
> I do take care to have the 'else' in the line just above 'if'.

It's unfortunate if this style has found its way into other places in our codebase.

> But whatever, if Greg is surprised by it, not many people will be using my
> way. Will fix, but now it's high time for bed.

I don't think it's a question of anybody being surprised, or one style is better than another or Greg being the bees-knees or anything like that. Long ago we picked a coding standard, and it just happened to be the "else if (..." style. I'm sure I don't have to enumerate the benefits to being consistent in our style, but suffice it to say it's the way The World is.

(FWIW, I personally prefer "if (...) {", but we don't do that, either. Old habits die hard.)

-Hyrum
Received on 2010-03-12 04:19:08 CET