+1 with all-at-once — *Joris Van Remoortere* Mesosphere
On Thu, Nov 5, 2015 at 9:37 AM, Kapil Arya <[email protected]> wrote: > +1. > > I think we should do it all at once as Artem mentioned. This doesn't really > affect the history (git-blame, etc.) because we are not touching code per > se. > > On Thu, Nov 5, 2015 at 12:29 PM, Artem Harutyunyan <[email protected]> > wrote: > > > Hi Alex, > > > > While I agree with the idea in general, I strongly believe that we should > > either leave it as it is or fix everything in one go (i.e. three > > consecutive commits). Having both #include guards and #pragmas in the > > codebase will be confusing and untidy. We have done code sweeps like this > > in the past when we had to introduce changes to the style guide, so if > > folks agree you just need to find a shepherd and do it :). > > > > Cheers, > > Artem. > > > > On Wed, Nov 4, 2015 at 9:36 PM, Alex Clemmer < > [email protected]> > > wrote: > > > > > Hey folks. > > > > > > In r/39803[1], Mike Hopcroft (in quintessential MSFT style, heh) > > > brought up the issue of moving away from #include guards and towards > > > `#pragma once`. > > > > > > As this has been brought up before, I will be brief: we think it's > > > revisiting because the primary objection in previous threads appears > > > to be that, though `#pragma once` is a cleaner solution to the > > > multiple-include problem, it's not so much better that it's worth the > > > code churn. However, the ongoing Windows integration work means we > > > have to touch these files anyway, so if we agree this is cleaner and > > > desirable, then this is an opportunity to obtain that additional code > > > clarity, without the cost of the churn. > > > > > > For the remainder of the email, I will summarize the history of our > > > discussion of this issue, who will do the work, and what the next > > > steps are. > > > > > > PROPOSAL: We propose that all new code use `#pragma once` instead of > > > #include guards; for existing files, we propose that you change > > > #include guards when you touch them. > > > > > > HISTORY: This has been discussed before, most recently a year ago on > > > the mailing list[2]. There is a relevant JIRA[3] and discarded > > > review[4] that changes style guide's recommendation on the matter. > > > > > > SUMMARIZED OBJECTIONS: > > > 1. The Google style guide explicitly forbids `#pragma once`. > > > 2. This results in a lot of code churn, but is only marginally better. > > > 3. It's not C++ standardized/it's platform dependent/IBM's compiler > > > doesn't support it. > > > 4. Popular projects like Chrome don't do `#pragma once` because of > > > history clutter. > > > 5. Intermediate state of inconsistency as we transition to `#pragma > > > once` from #include guards. > > > > > > OUR RESPONSE: > > > Objections (1), (2), and (4) are essentially the same -- Dominic Hamon > > > points out in a previous thread that the Google style guide was > > > canonized when `#pragma once` was Windows-only, and the guidance has > > > not changed since because of the history churn problem. As noted > > > above, we think the history churn problem is minimized by the fact > > > that it can be wrapped up into the Windows integration work. > > > > > > For objection (3), the consensus seems to be that the vast majority of > > > compilers we care about (in particular, the ones supporting C++ 11) do > > > support it. > > > > > > For objection (5) we believe the inconsistent state is likely to not > > > be long lived, as long as we commit to wrapping this work up into the > > > Windows integration work. > > > > > > SUMMARIZED ADVANTAGES: > > > * Basically fool-proof. Communicates simply what its function is (you > > > include this file once). Semantically it is "the right tool for the > > > job". > > > * No need for namespacing conventions for #include guards. > > > * No conflicts with reserved identifiers[5]. > > > * No internal conflicts between include guards in Stout, Process > > > library, and Mesos (this is one reason we need the namespacing > > > conventions) > > > * Reduces preprocessor definition clutter (we should rely on #define > > > as little as humanly possible). > > > * Optimized to be easy to read and reason about. > > > > > > NEXT STEPS: > > > If we agree that this is the right thing to do, committers would ask > > > people to use `#pragma once` for new code when presented in code > > > reviews. For files that exist, I will take point on transitioning as > > > we complete the Windows integration work. I expect this work to > > > completely land before the new year. > > > > > > > > > Thanks, > > > > > > > > > [1] https://reviews.apache.org/r/39803/ > > > [2] https://www.marc.info/?t=142540100400015&r=1&w=2 > > > [3] https://issues.apache.org/jira/browse/MESOS-2211 > > > [4] https://reviews.apache.org/r/30100/ > > > [5] > > > > > > http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier > > > > > > > > > -- > > > Alex > > > > > > Theory is the first term in the Taylor series of practice. -- Thomas M > > > Cover (1992) > > > > > >
