On Wed, Jan 21, 2015 at 2:20 PM, Benjamin Mahler <[email protected]> wrote:
> It's really great to see the enthusiasm to improve the code! Much like Ben, > I would love to see a more complete depiction of the pros and cons before > we go ahead and push things like this, especially as we have a growing set > of committers. > > Why is it that many projects are not adopting #pragma once? E.g. Chromium: > http://dev.chromium.org/developers/coding-style#TOC-File-headers > > Why is it that the Google style guide forbids it for Windows code? > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html?showone=Windows_Code#Windows_Code In both these cases it is the burden of history. If it's not worth the churn in the codebase (and it's probably not) then it's certainly not worth much more discussion. However, we should change something given that the existing include guards don't follow the language spec (underscore followed by uppercase) and are inconsistent. This change is as much churn on the codebase as a change to pragma, and pragma gets us away from any naming bikeshedding. > > And accordingly, if there is a Windows reason, things like stout and > libprocess might not want to use #pragma. > > On Wed, Jan 21, 2015 at 10:01 AM, John Pampuch <[email protected]> wrote: > > > Wikipedia has a list of significant compilers that support. IBM's is the > > only one. > > > > I'm not too concerned about IBM's. And I'm not discouraging this, I just > > want to make sure we go in with both eyes open. > > > > -John > > > > > > On Wed, Jan 21, 2015 at 2:31 AM, Alexander Rojas < > [email protected]> > > wrote: > > > > > While I understand your concern, I think #pragma once reduces clutter > in > > > the code and improves readability which I think is the important thing > > > here. However I just see portability could be an issue (however almost > > all > > > mayor compilers support this pragma nowadays). > > > > > > Do we in general have a list of supported compilers? It would be sad > not > > > to go for this patch because we cannot get compiled in IBM’s compiler > > only > > > to find out later on that mesos never compiled there anyway. > > > > > > - Alexander > > > > > > > > > > On 21 Jan 2015, at 07:31, John Pampuch <[email protected]> wrote: > > > > > > > > Not to be too much of a stickler, but #pragma once is a bit strange. > > It > > > > runs slightly counter to: > > > > > > > > The ‘#pragma’ directive is the method specified by the C standard for > > > > providing additional information to the compiler, beyond what is > > conveyed > > > > in the language itself. > > > > > > > > > > > > I suppose a directive that says "skip this file under certain > > conditions" > > > > is "beyond what is conveyed in the language itself". The problem is > > that > > > > code that compiles fine with the pragma could fail (fairly easily) > > > without, > > > > which doesn't seem to be the intent of pragmas. > > > > > > > > But conceptually, it does seem like a better solution. Of course, > > > > compliers aren't obligated to support it, so any compiler that > probably > > > > won't work. > > > > > > > > It probably doesn't matter much, but IBM's commercial XL C/C++ > compiler > > > > doesn't support this per the venerable Wikipedia. > > > > > > > > -John > > > > > > > > > > > > On Tue, Jan 20, 2015 at 6:08 PM, Kapil Arya <[email protected]> > > wrote: > > > > > > > >> On Tue, Jan 20, 2015 at 6:00 PM, Benjamin Hindman < > > > [email protected]> > > > >> wrote: > > > >> > > > >>> What's the rush on committing this? Let's make sure all of the > > > committers > > > >>> get a chance to share their opinion on this please. > > > >>> > > > >> > > > >> This was assuming that most people would agree on the change. > > However, > > > if > > > >> that's not the case, then there is no hurry :-). > > > >> > > > >> > > > >>> I for one would love to hear from others that have used #pragma > once > > in > > > >>> practice and hear any pros/cons from them. > > > >>> > > > >>> Also, while the > > to >> change was meant to preserve some > > > information, > > > >>> I'm not convinced that there is any information to preserve by > > > replacing > > > >>> all include guards with #pragma once, and otherwise I feel like > we're > > > >> going > > > >>> to get just as many reviews where people have to tell you to switch > > to > > > >>> #pragma once rather than appropriately name the include guard. > > > >>> > > > >> > > > >> Good point. In this case, we can just do a mass update (if we agree > to > > > the > > > >> change). > > > >> > > > >>> > > > >>> On Tue, Jan 20, 2015 at 5:04 PM, Kapil Arya <[email protected]> > > > wrote: > > > >>> > > > >>>> Hi All, > > > >>>> > > > >>>> This issue came up on several occasions. Since people seem to > agree > > on > > > >>>> using "#pragma once" instead of "#define" guards that we have > > using, I > > > >>>> wanted to send a quick email to gather some consensus around it. > If > > > >>>> everyone agrees about the switch, we can update the style guide > and > > > >> start > > > >>>> using "#pragma once". > > > >>>> > > > >>>> The issue is tracked at > > > >> https://issues.apache.org/jira/browse/MESOS-2211 > > > >>>> and there is a review request at > > https://reviews.apache.org/r/30100/. > > > >>>> > > > >>>> Please note that this won't be a mass update. We should keep > > updating > > > >> the > > > >>>> files that are created/updated (similar in spirit to "> >" to ">>" > > > >>> change). > > > >>>> > > > >>>> If there are any concerns, please let us know. We would very much > > like > > > >> to > > > >>>> commit this by tomorrow and any feedback before that is highly > > > >>> appreciated. > > > >>>> > > > >>>> Best, > > > >>>> Kapil > > > >>>> > > > >>> > > > >> > > > > > > > > > -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
