Re: CodingStyle on existing code

[Wido den Hollander <wido@42on.com>]

On 12/03/2015 12:12 PM, Joao Eduardo Luis wrote:
> On 12/01/2015 03:18 PM, Sage Weil wrote:
>> On Tue, 1 Dec 2015, Wido den Hollander wrote:
>>>
>>> On 01-12-15 16:00, Gregory Farnum wrote:
>>>> On Tue, Dec 1, 2015 at 5:47 AM, Loic Dachary <loic@dachary.org> wrote:
>>>>>
>>>>>
>>>>> On 01/12/2015 14:10, Wido den Hollander wrote:
>>>>>> Hi,
>>>>>>
>>>>>> While working on mon/PGMonitor.cc I see that there is a lot of
>>>>>> inconsistency on the code.
>>>>>>
>>>>>> A lot of whitespaces, indentation which is not correct, well, a lot of
>>>>>> things.
>>>>>>
>>>>>> Is this something we want to fix? With some scripts we can probably do
>>>>>> this easily, but it might cause merge hell with people working on features.
>>>>>
>>>>> A sane (but long) way to do that is to cleanup when fixing a bug or adding a feature. With (a lot) of patience, it will eventually be better :-)
>>>>
>>>> Yeah, we generally want you to follow the standards in any new code. A
>>>> mass update of the code style on existing code makes navigating the
>>>> history a little harder so a lot of people don't like it much, though.
>>>
>>> Understood. But in this case I'm working in PGMonitor.cc. For just 20
>>> lines of code I probably shouldn't refactor the whole file, should I?
> 
> While it annoys me finding a given commit in history that just changes
> every single line to fix styling, I also recognize that this is the sort
> of janitorial task that may be warranted for certain files.
> 
> As sage mentions below, this is a low-traffic file. And whenever it's
> changed, is often just tiny bits here and there. That tends to add to
> the style divergence rather than to convergence.
> 
> I'd say go for it. If you are indeed changing those 20 or so lines
> though, please add those lines on a separate patch from the style changes.
> 

Here is the first pull request without a functional change. It is just
the codingstyle fix: https://github.com/ceph/ceph/pull/6881

If that comes through I can put in the actual code.

Wido

>   -Joao
> 
>>
>> Easiest thing is to fix the code around your change.
>>
>> I'm also open to a wholesale cleanup since it's a low-traffic file and 
>> likely won't conflict with other stuff in flight.  But, up to you!
>>
>> sage
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 


-- 
Wido den Hollander
42on B.V.
Ceph trainer and consultant

Phone: +31 (0)20 700 9902
Skype: contact42on