ome otherwise very-readable code.
>
>From: Blake Bender
>Date: Monday, May 3, 2021 at 11:23 AM
> To: 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 con
eode.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 c
> On May 4, 2021, at 7:59 AM, Mario Salazar de Torres
> wrote:
>
> Sure, gladly 🙂. Let me put up the info and I will open the discussion.
> And sorry for polluting this topic regarding Style and Formatting :S
No apologies! I just think this topic has a much bigger scope than documenting
our
: Geode Native C++ Style and Formatting Guide
Mario,
Let’s start a separate discussion for going to C++17 on a minor release. Can
you lay out the pros and cons to kick it off?
> On May 4, 2021, at 7:31 AM, Mario Salazar de Torres
> wrote:
>
> Hi everyone,
>
> Regarding Blake&#
dard for the project as it has
> several cool features, but sadly I must admit is too recent to be adopted.
>
> Thanks,
> Mario.
>
> From: Blake Bender
> Sent: Monday, May 3, 2021 8:23 PM
> To: dev@geode.apache.org
> Subject: RE: DISCU
ION: 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 h
> On May 3, 2021, at 11:23 AM, Blake Bender wrote:
>
> 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
This is likely an ABI change and certainly moves the minimum runtime library.
The latter might be hard in a minor release.
>
ng-tidy?) of mangling some otherwise very-readable code.
>
> From: Blake Bender
> Date: Monday, May 3, 2021 at 11:23 AM
> To: 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 con
80 characters also feels arbitrary, especially with auto-formatters
(clang-tidy?) of mangling some otherwise very-readable code.
From: Blake Bender
Date: Monday, May 3, 2021 at 11:23 AM
To: dev@geode.apache.org
Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide
My $0.02 on
elevant 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
Sent: Saturday, May 1, 2021 9:21 AM
To: dev@geode.apache.org
Subject: Re: DISCUSSION: Geode Native C++
Great call outs!
> On May 1, 2021, at 7:57 AM, Mario Salazar de Torres
> 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 w
only used when there is no alternative"?
Thanks,
Mario.
From: Jacob Barrett
Sent: Saturday, May 1, 2021 2:31 AM
To: dev@geode.apache.org
Subject: DISCUSSION: Geode Native C++ Style and Formatting Guide
Team,
We haven’t been good about documenting our style
Team,
We haven’t been good about documenting our style and formatting guidance. Like
the Java sources are loosely styled after the Google Java Style Guide, the C++
sources are loosely styled after the Google C++ Style Guide. We deviate in some
places from the Google C++ Style Guide. Without
Github user asfgit closed the pull request at:
https://github.com/apache/geode-native/pull/32
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103054588
--- Diff: src/cppcache/integration-test/BuiltinCacheableWrappers.hpp ---
@@ -23,7 +23,7 @@
#include "CacheableWrapper.hpp"
extern "C"
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103054617
--- Diff: src/cppcache/src/CacheableBuiltins.cpp ---
@@ -19,7 +19,7 @@
#include
extern "C" {
-#include
+#include
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103045677
--- Diff: src/tests/cpp/fwklib/Timer.hpp ---
@@ -28,18 +28,18 @@
#ifdef _WIN32
-#include
-#include
-#include
+#inclu
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103044451
--- Diff: src/cppcache/src/Utils.cpp ---
@@ -22,7 +22,7 @@
#include
extern "C" {
-#include
+#include
--- End diff --
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103031314
--- Diff: src/tests/cpp/fwklib/Timer.hpp ---
@@ -28,18 +28,18 @@
#ifdef _WIN32
-#include
-#include
-#include
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103031038
--- Diff: src/cppcache/src/Utils.cpp ---
@@ -22,7 +22,7 @@
#include
extern "C" {
-#include
+#include
--- End diff
GitHub user dgkimura reopened a pull request:
https://github.com/apache/geode-native/pull/32
GEODE-2439: Replace c-style headers with c++ headers
Purpose is to remove potentially non-portable c headers with the c++
equivalent headers.
Testing - built on OSX
You can merge
Github user dgkimura commented on the issue:
https://github.com/apache/geode-native/pull/32
@echobravopapa Yessir
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wish
Github user dgkimura closed the pull request at:
https://github.com/apache/geode-native/pull/32
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103009496
--- Diff: src/tests/cpp/security/PkcsAuthInit.cpp ---
@@ -19,7 +19,7 @@
#include
#include
#include
-#include "stdio.h"
+#inclu
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103009417
--- Diff: src/tests/cpp/security/XmlAuthzCredentialGenerator.hpp ---
@@ -27,7 +27,7 @@
#include
#include
--- End diff --
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103009272
--- Diff: src/cppcache/test/ByteArray.cpp ---
@@ -20,7 +20,7 @@
#include "config.h"
#ifdef _MACOSX
-#include // For wcstom
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103009143
--- Diff: src/tests/cpp/fwklib/TaskClient.cpp ---
@@ -20,12 +20,12 @@
#include "fwklib/FwkLog.hpp"
#include "fwklib/PerfFwk.hpp"
-#i
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103008731
--- Diff: src/cppcache/test/ByteArray.cpp ---
@@ -20,7 +20,7 @@
#include "config.h"
#ifdef _MACOSX
-#include // For wcstombs()
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103008420
--- Diff: src/cppcache/test/ByteArray.cpp ---
@@ -20,7 +20,7 @@
#include "config.h"
#ifdef _MACOSX
--- End diff --
Yeah, se
Github user dgkimura commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103008324
--- Diff: src/cppcache/src/statistics/HostStatHelperWin.hpp ---
@@ -27,10 +27,10 @@
#include
#include
-#include
-#include
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103005713
--- Diff: src/cppcache/src/statistics/HostStatHelperWin.hpp ---
@@ -27,10 +27,10 @@
#include
#include
-#include
-#inc
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103004805
--- Diff: src/cppcache/test/ByteArray.cpp ---
@@ -20,7 +20,7 @@
#include "config.h"
#ifdef _MACOSX
--- End diff --
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103005160
--- Diff: src/tests/cpp/fwklib/TaskClient.cpp ---
@@ -20,12 +20,12 @@
#include "fwklib/FwkLog.hpp"
#include "fwklib/PerfFwk.hpp"
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103005494
--- Diff: src/tests/cpp/security/XmlAuthzCredentialGenerator.hpp ---
@@ -27,7 +27,7 @@
#include
#include
--- End diff --
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103004766
--- Diff: src/cppcache/test/ByteArray.cpp ---
@@ -20,7 +20,7 @@
#include "config.h"
#ifdef _MACOSX
-#include // For wcstom
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/32#discussion_r103005369
--- Diff: src/tests/cpp/security/PkcsAuthInit.cpp ---
@@ -19,7 +19,7 @@
#include
#include
#include
-#include "stdio.h"
Github user echobravopapa commented on the issue:
https://github.com/apache/geode-native/pull/32
@dgkimura STABLE tests passing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
e
GitHub user dgkimura opened a pull request:
https://github.com/apache/geode-native/pull/32
GEODE-2439: Replace c-style headers with c++ headers
Purpose is to remove potentially non-portable c headers with the c++
equivalent headers.
Testing - built on OSX
You can merge
t) should be used in cases
>> where the value may be entirely absent (e.g., there is no foo) and
>> references (both const and non-const) used in all other cases. There may be
>> special cases (e.g., mocks for Google Test) where a pointer to an interface
>> is preferable but
o) and
> references (both const and non-const) used in all other cases. There may be
> special cases (e.g., mocks for Google Test) where a pointer to an interface
> is preferable but as a general rule the precondition that a reference can
> not be null makes them preferable.
>
> Sarg
pecial cases (e.g., mocks for
Google Test) where a pointer to an interface is preferable but as a general
rule the precondition that a reference can not be null makes them preferable.
Sarge
> On 20 Feb, 2017, at 13:48, Jacob Barrett wrote:
>
> A bit back we discussed and adopted th
A bit back we discussed and adopted the Google C++ Style Guide. As we dig
deeper into the C++ sources we find some striking differences in some of
the conventions that we may want to discuss and address before tackling
them.
*Variable Naming*
Google style dictates no *camelCase*, use either
42 matches
Mail list logo
