On 12/09/2015 11:16 PM, Wido den Hollander wrote: > 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@xxxxxxxxxxx> 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. Seems a whole lot less intrusive than I expected. :) -Joao > > 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@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
