You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/07/29 10:07:56 UTC
[GitHub] [nifi-minifi-cpp] szaszm opened a new pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
szaszm opened a new pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146
- divergence from the Google Style Guide
- recommendation to follow the C++ Core Guidelines
This is largely based on my observation of existing practice to help new contributors. If you don't agree with any of the changes, would like to add or remove something, please leave a comment to discuss the issue.
Preview the rendered output [here](https://github.com/szaszm/nifi-minifi-cpp/blob/MINIFICPP-1617/CONTRIB.md).
---
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [x] If applicable, have you updated the LICENSE file?
- [x] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
martinzink commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679908862
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,70 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the
+[Google C++ Style Guide][https://google.github.io/styleguide/cppguide.html], except for the following points:
Review comment:
Now I see I used wrong syntax for the link in my suggestion. sry
```suggestion
[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html), except for the following points:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679706231
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I would still prefer some indication in this section that operator overloading can easily result in those later-mentioned footguns, but I'm OK with the current version if everyone else is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679054962
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
Did you mean `static_assert` instead of `if constexpr`? Or `std::conditional` instead of `std::enable_if`? Because they serve different purposes.
I don't want to discourage TMP too much. Of course `constexpr` is preferred for values, but TMP allows some great interfaces and libraries to exist. One such example is the standard ranges library.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
martinzink commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679825979
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,60 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
Review comment:
We could link to the actual google style guide here.
```suggestion
New contributions are expected to follow the [Google C++ Style Guide][https://google.github.io/styleguide/cppguide.html], except for the following points:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679592382
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I came up with a slightly different version in 3fd87be. I'm hoping for to reach consensus, let me know if you like it or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679071529
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
Operator overloading can be nice, especially if done well in a library. But it can easily be done poorly, and it does make navigating the code harder. So I would like to lightly discourage it, as above, but I'd be interested in what others think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679056562
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
+- Static data members are capitalized according to lower_snake_case, in contrast to the Google Style Guide, which uses
Review comment:
I'll remove the line then. That should mean fallback to the Google Style Guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679592512
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
I came up with a slightly different version in 3fd87be. I'm hoping for to reach consensus, let me know if you like it or not.
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I came up with a slightly different version in 3fd87be. I'm hoping to reach consensus, let me know if you like it or not.
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
I came up with a slightly different version in 3fd87be. I'm hoping to reach consensus, let me know if you like it or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits closed pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits closed pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679026918
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
Suggested addition:
```suggestion
- Operator overloading and user-defined literal suffixes are OK, but use them sparingly and only when they make the code more readable
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679052318
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I don't want to discourage their usage. Using operator-overloading even for syntactic sugar is fine by my standards, it makes great interfaces like [Clara](https://github.com/catchorg/Clara) possible. And UDLs can also be helpful, see Literals.h in our codebase or the [units](https://github.com/nholthaus/units) library. Of course they can be misused, but I'd rather catch those during reviews than to potentially prevent their benefits from being utilized because of the discouraging wording.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679201307
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I'm okay with that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679198370
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
What do you think about not adding anything here and adding a sentence in the text part, emphasizing simplicity and readability? Like "Contributions should aim for simple and readable code".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679747713
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
Let's leave this open so that future reviewers can see and join the bikeshedding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679208828
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
I think the "Prefer" part is a very specific use case and falls under the general goal of simple code anyway. I agree with the proposition, but wouldn't want to include it in the doc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679704052
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
Yes, I like the new version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679198370
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
What do you think about not adding anything here, but adding a sentence in the text part, emphasizing simplicity and readability? Like "Contributions should aim for simple and readable code".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#issuecomment-889814075
> Nice work. +1 There is a Contributing part in README.md as well I think that could be just a link to this file to eliminate the need to update both of them.
> And just include the shellscript, python part (which is only mentioned there atm) here.
Moved the info over to CONTRIB.md in f8413dc. Also added a reference to `make shellcheck`, as I think that's the easiest way to invoke it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679067097
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
I was thinking of examples like https://www.bfilipek.com/2018/03/ifconstexpr.html, where `if constexpr` can simplify code which would have been written using `std::enable_if` before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679024480
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
Review comment:
typo:
```suggestion
class. UpperCamelCase is used for most classes, in line with the Google Style Guide.
```
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
Review comment:
Suggested addition:
```suggestion
- Template metaprogramming is OK where it is necessary, as long as the usage is clear. Prefer `if constexpr` to `std::enable_if`.
```
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
+- Static data members are capitalized according to lower_snake_case, in contrast to the Google Style Guide, which uses
Review comment:
I think our current usage of static (non-constant) data members is mixed, both with trailing `_` and without. I would prefer having the trailing `_` in new code, as I would rather distinguish static members from local variables than from non-static members.
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
Suggested addition:
```suggestion
- Operator overloading and user-defined literal suffixes are OK, but use them sparingly and only where they make the code more readable
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits closed pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits closed pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679167560
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
I'm not a fan of operator overloading, but it is because of my own bad experiences. But I think my own bad experiences shouldn't discourage anyone who would create an excellent interface through operator overloading. I wouldn't add the `but use them sparingly` part, and I would rather say `but only use them if they do not make the code less readable`. In that case if we catch a similar problem on review we can point to this part of the CONTRIB.md.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679745752
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
Review comment:
How about removing the Operator overloading part, falling back on the [style guide](https://google.github.io/styleguide/cppguide.html#Operator_Overloading), which says "Overload operators judiciously"? It also disallows overloading &&, ||, , (comma), and unary &, which seems fine to me, and there is always a possible fallback to this in extra special cases: "It's ok to diverge from any of the rules with a good enough reason.".
```suggestion
- User-defined literal suffixes are OK
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679591863
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- Operator overloading and user-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK as long as the usage is clear.
+- Static data members are capitalized according to lower_snake_case, in contrast to the Google Style Guide, which uses
Review comment:
done in 3fd87be
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,59 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the Google Style Guide, except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperSnakeCase is used for most classes, in line with the Google Style Guide.
Review comment:
done in 3fd87be
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1146: MINIFICPP-1617 Update CONTRIB.md with current practice
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1146:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1146#discussion_r679946074
##########
File path: CONTRIB.md
##########
@@ -15,24 +15,70 @@
# Apache NiFi - MiNiFi - C++ Contribution Guide
-
-We welcome all contributions to Apache MiNiFi. To make development easier, we've included
-the linter for the Google Style guide. Google provides an Eclipse formatter for their style
-guide. It is located [here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
-New contributions are expected to follow the Google style guide when it is reasonable.
-Additionally, all new files must include a copy of the Apache License Header.
-
+We welcome all contributions to Apache MiNiFi. All new files must include a copy of the Apache License Header.
+To make development easier, we've included the linter for the Google Style guide. Google provides an Eclipse formatter
+for their style guide. It is located
+[here](https://github.com/google/styleguide/blob/gh-pages/eclipse-cpp-google-style.xml).
+New contributions are expected to follow the
+[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html), except for the following points:
+- Use .cpp extension for implementation files
+- Use lowerCamelCase for functions, including accessors/mutators
+- Use UPPER_SNAKE_CASE for constants
+- Filenames are typically class names or a description of the contents in UpperCamelCase
+- If a class is imitating something from STL, boost or similar, then STL-style lower_snake_case is used for naming the
+ class. UpperCamelCase is used for most classes, in line with the Google Style Guide.
+- Prefer `#pragma once` over include guards
+- Forward declarations are OK
+- Using-directives (`using namespace foo`) are discouraged, except for user-defined literal namespaces
+- Some patterns in the codebase rely on objects with static storage duration without being trivially destructible and
+ initialized with a constant expression. It's OK to use these.
+- User-defined literal suffixes are OK
+- Public mutable data members are allowed
+- Inline function definition is OK
+- Rvalue references, exceptions and RTTI are allowed
+- Use gsl::narrow and gsl::narrow_cast in addition to standard casts. The codebase doesn't use abseil.
+- We are more liberal regarding the use of `auto`. The Google Style Guide only allows using it when it makes the code
+ clearer. In MiNiFi C++, it's up to the personal preferences of the contributor.
+- Template metaprogramming is OK when necessary, as long as the usage is clear. Prefer the more readable alternatives
+ when applicable.
+- Enums are either UPPER_SNAKE_CASE or UpperCamelCase.
+- File-level comments describing the contents are not required and typically not used in the codebase. A license header
+ is required.
+- Function comments use /** Javadoc style */
+- Line length is not limited, but the linter warns on lines longer than 200 characters. Use a NOLINT line comment in the
+ rare case when a longer line is more readable than splitting it up.
+- Continuation indentation is ok with either 2 levels of indentation (4 spaces) or aligned.
+
+It's ok to diverge from any of the rules with a good enough reason. We recommend following the C++ Core Guidelines, when
+it doesn't contradict any of the Google Style Guide or the above exceptions.
+
+C++ is a powerful language and "with great power comes great responsibility". Please aim for simple, readable and
+maintainable solutions and avoid footguns. Be open for respectful debate in pull request reviews.
+
+Shell script files shall follow the guidelines and best practices defined by the [shellcheck](https://github.com/koalaman/shellcheck) analysis tool.
+New contributions are expected to pass the shellcheck analysis as part of the verification process.
+If a shellcheck requested change is unfeasable it shall be disabled on per-line basis and will be subjected to review.
Review comment:
typo:
```suggestion
If a shellcheck requested change is unfeasible it shall be disabled on per-line basis and will be subjected to review.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org