What are we missing/unable to format with our clang-tools?
These style discussions tend to become holy wars, hopefully we can avoid this...
if we can tool are largest concerns and perform good PR reviews looking for
valid algorithms, good mnemonics naming variables and such I think we'll be
doing just fine.
EB
On 5/3/21, 2:07 PM, "Robert Houghton" <rhough...@vmware.com> wrote:
80 characters also feels arbitrary, especially with auto-formatters
(clang-tidy?) of mangling some otherwise very-readable code.
From: Blake Bender <bbl...@vmware.com>
Date: Monday, May 3, 2021 at 11:23 AM
To: dev@geode.apache.org <dev@geode.apache.org>
Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide
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
