On Mar 11, 2010, at 9:11 PM, Neels J Hofmeyr wrote:
> Greg Stein wrote:
>> On Thu, Mar 11, 2010 at 08:52, <ne...@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
