My $0.02 on these: Things I'd like to see us conform to Google style on: * I'd be happy to move to C++ 17 * Would also be happy to remove forward declarations. "I'm not a critic, but I know what I hate," as it were, and I hate forward declarations. * I would also be happy with an 80-character line limit, though I don't feel strongly about it. 100 may be consistent with Geode, but it still feels arbitrary to me. * I would be very pleased to remove all the macros from our code. I've been bitten more than once in the past while debugging or refactoring our code, because of ill-formed macros.
Google things I disagree with: * I don't like exceptions, but I don't even want to think about the amount of effort required to remove them from the codebase is, IMO, unreasonably high. Keep the exceptions, most of the time they're used pretty judiciously. * I really, really, *really* (really? Yes, really!) hate anything resembling Hungarian prefix notation, and have permanent scars from decades of reading it in Windows code. Please don't ask me to put a random 'k' in from of my enums - ick. One other note: in the past, we've had conversations about "style only" pull requests to fix some of these things, and the guidance we ended up with has been to only fix this sort of thing while you're in the code working on a fix or a feature. I, for one, would welcome some PRs that just, say, renamed a ton of member variables to replace "m_" prefix with a simple trailing "_", perhaps fixed some of the more egregious and weird abbreviations, etc. My preference for bug fixes and feature work is that all of the code changes be focused on stuff that's relevant to the fix/feature, and mixing it with random style guide refactoring, I feel, muddies the waters for future maintainers. Thanks, Blake -----Original Message----- From: Jacob Barrett <jabarr...@vmware.com> Sent: Saturday, May 1, 2021 9:21 AM To: dev@geode.apache.org Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide Great call outs! > On May 1, 2021, at 7:57 AM, Mario Salazar de Torres > <mario.salazar.de.tor...@est.tech> wrote: > > 1. Member variables names as of Google style guide requires a '_' char to > be added at the end so it can be identified. Should we also adopt that? > For example, imagine you have a region mutex, so, should we name it as > 'regionMutex_' ? I didn’t mention this one out in my review of differences because we are following it but I suppose with the combination of the camelCase difference we should probably call it out more specifically. Perhaps in our documentation we should show examples of both local and member variables. Do you think that will make it more clear? > 2. Also, I would like to point out that macros are dis-recommended but > every C++ committee member I know. > What do you think about adding a notice saying: "Macros should be avoided and > only used when there is no alternative”? I think that is called out in various ways in a few places in the Google guide but I am more than happy for us to include strong or clearer language around this. Between constexpr and templates there are very cases for macros anymore. We mostly use macros only to handle non-standard attributes. When we move to C++17 a lot of these will go away. Thanks, Jake
