szaszm commented on a change in pull request #789:
URL: https://github.com/apache/nifi-minifi-cpp/pull/789#discussion_r432462298
##########
File path: generateVersion.sh
##########
@@ -37,8 +37,25 @@ IFS=';' read -r -a extensions_array <<< "$extensions"
extension_list="${extension_list} } "
cat >"$out_dir/agent_version.h" <<EOF
-#ifndef AGENT_BUILD_H
-#define AGENT_BUILD_H
+/**
Review comment:
I would rather exclude the generated file from the linter checks. I
think the copyrighted material here is the generator script, but not the
generated file.
##########
File path: libminifi/include/c2/triggers/FileUpdateTrigger.h
##########
@@ -38,8 +41,7 @@ namespace c2 {
*/
class FileUpdateTrigger : public C2Trigger {
public:
-
- FileUpdateTrigger(std::string name, utils::Identifier uuid =
utils::Identifier())
+ explicit FileUpdateTrigger(std::string name, utils::Identifier uuid =
utils::Identifier())
Review comment:
This breaks API
##########
File path: libminifi/include/core/Core.h
##########
@@ -20,11 +20,12 @@
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN 1
#endif
+#include <properties/Configure.h>
Review comment:
We should probably change this to use quotation marks as it's not a
system header. This would also change its position, that's why I suggest doing
it in context of this PR.
##########
File path: extensions/standard-processors/processors/GetTCP.h
##########
@@ -26,7 +30,7 @@
#include "core/ProcessSession.h"
#include "core/Core.h"
#include "core/Resource.h"
-#include "concurrentqueue.h"
+#include "concurrentqueue.h" // NOLINT
Review comment:
What was the issue here?
##########
File path: libminifi/include/core/Resource.h
##########
@@ -1,68 +1,70 @@
-/**
- *
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#ifndef LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
-#define LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
-
-#ifndef WIN32_LEAN_AND_MEAN
-#define WIN32_LEAN_AND_MEAN 1
-#endif
-
-#include "ClassLoader.h"
-#include "agent/agent_docs.h"
-
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace core {
-
-#define MKSOC(x) #x
-#define MAKESTRING(x) MKSOC(x)
-
-template<class T>
-class StaticClassType {
- public:
-
- StaticClassType(const std::string &name, const std::string &description =
"") {
- // Notify when the static member is created
- if (!description.empty()){
- minifi::AgentDocs::putDescription(name,description);
- }
-#ifdef MODULE_NAME
- ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new
DefautObjectFactory<T>(MAKESTRING(MODULE_NAME))));
-#else
- ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new DefautObjectFactory<T>("minifi-system")));
-#endif
- }
-};
-
-#define REGISTER_RESOURCE(CLASSNAME,DESC) \
- static core::StaticClassType<CLASSNAME> \
- CLASSNAME##_registrar( #CLASSNAME, DESC );
-
-#define REGISTER_RESOURCE_AS(CLASSNAME,NAME) \
- static core::StaticClassType<CLASSNAME> \
- CLASSNAME##_registrar( #NAME );
-
-}/* namespace core */
-} /* namespace minifi */
-} /* namespace nifi */
-} /* namespace apache */
-} /* namespace org */
-
-#endif /* LIBMINIFI_INCLUDE_CORE_RESOURCE_H_ */
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
+#define LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
+
+#include <memory>
+#include <string>
+
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN 1
+#endif
+
+#include "ClassLoader.h"
+#include "agent/agent_docs.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+#define MKSOC(x) #x
+#define MAKESTRING(x) MKSOC(x)
+
+template<class T>
+class StaticClassType {
+ public:
+ explicit StaticClassType(const std::string &name, const std::string
&description = "") {
+ // Notify when the static member is created
+ if (!description.empty()) {
+ minifi::AgentDocs::putDescription(name, description);
+ }
+#ifdef MODULE_NAME
+ ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new
DefautObjectFactory<T>(MAKESTRING(MODULE_NAME))));
+#else
+ ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new DefautObjectFactory<T>("minifi-system")));
+#endif
+ }
+};
+
+#define REGISTER_RESOURCE(CLASSNAME, DESC) \
+ static core::StaticClassType<CLASSNAME> \
+ CLASSNAME##_registrar(#CLASSNAME, DESC);
+
+#define REGISTER_RESOURCE_AS(CLASSNAME, NAME) \
+ static core::StaticClassType<CLASSNAME> \
+ CLASSNAME##_registrar(#NAME);
+
+}/* namespace core */
+} // namespace minifi
+} // namespace nifi
+} // namespace apache
+} // namespace org
Review comment:
`core` is still the wrong style. It's likely to have escaped your regex
because of the lack of space before the comment.
##########
File path: libminifi/include/core/WeakReference.h
##########
@@ -87,17 +85,15 @@ class ReferenceContainer {
}
protected:
-
std::mutex mutex;
std::vector<std::shared_ptr<WeakReference> > references;
-
};
}/* namespace core */
-} /* namespace minifi */
-} /* namespace nifi */
-} /* namespace apache */
-} /* namespace org */
+} // namespace minifi
+} // namespace nifi
+} // namespace apache
+} // namespace org
Review comment:
`core` should be fixed, too
##########
File path: libminifi/include/core/state/nodes/AgentInformation.h
##########
@@ -72,7 +76,7 @@ class ComponentManifest : public DeviceInformation {
: DeviceInformation(name, uuid) {
}
- ComponentManifest(const std::string &name)
+ explicit ComponentManifest(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/c2/C2Callback.h
##########
@@ -18,17 +18,17 @@
#ifndef LIBMINIFI_INCLUDE_C2_C2CALLBACK_H_
#define LIBMINIFI_INCLUDE_C2_C2CALLBACK_H_
+#include <memory>
+
#include "C2Agent.h"
/**
* Purpose: Provides a callback with very specific accesses
*/
class C2Callback {
-
public:
- C2Callback(const std::shared_ptr<C2>)
-
+ explicit C2Callback(const std::shared_ptr<C2>)
Review comment:
This breaks API
##########
File path: libminifi/include/core/ClassLoader.h
##########
@@ -81,10 +82,8 @@ class ObjectFactoryInitializer {
* creating processors from shared objects.
*/
class ObjectFactory {
-
public:
-
- ObjectFactory(const std::string &group)
+ explicit ObjectFactory(const std::string &group)
Review comment:
This breaks API, but I think is unlikely to be used outside of libminifi
##########
File path: libminifi/include/c2/ControllerSocketProtocol.h
##########
@@ -35,11 +40,9 @@ namespace c2 {
*/
class ControllerSocketProtocol : public HeartBeatReporter {
public:
-
- ControllerSocketProtocol(std::string name, utils::Identifier uuid =
utils::Identifier())
+ explicit ControllerSocketProtocol(std::string name, utils::Identifier uuid =
utils::Identifier())
Review comment:
This breaks API
##########
File path: libminifi/include/c2/PayloadParser.h
##########
@@ -174,8 +174,7 @@ class PayloadParser {
PayloadParser(PayloadParser &&parser) = default;
private:
-
- PayloadParser(const C2Payload &payload)
+ explicit PayloadParser(const C2Payload &payload)
Review comment:
This breaks API
##########
File path: libminifi/include/c2/PayloadParser.h
##########
@@ -30,10 +33,9 @@ namespace c2 {
class PayloadParseException : public std::runtime_error {
public:
- PayloadParseException(const std::string &msg)
+ explicit PayloadParseException(const std::string &msg)
Review comment:
This breaks API, although I think it's unlikely to be used outside of
libminifi
##########
File path: libminifi/include/core/state/nodes/BuildInformation.h
##########
@@ -65,12 +68,11 @@ namespace response {
*/
class BuildInformation : public DeviceInformation {
public:
-
BuildInformation(std::string name, utils::Identifier &uuid)
: DeviceInformation(name, uuid) {
}
- BuildInformation(const std::string &name)
+ explicit BuildInformation(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/RemoteProcessorGroupPort.h
##########
@@ -46,7 +49,7 @@ namespace minifi {
*/
class RPGLatch {
public:
- RPGLatch(bool increment = true) {
+ explicit RPGLatch(bool increment = true) {
Review comment:
This breaks API
##########
File path: libminifi/include/io/NonConvertingStream.h
##########
@@ -36,18 +39,16 @@ namespace io {
* Extensions may be thread safe and thus shareable, but that is up to the
implementation.
*/
class NonConvertingStream : public BaseStream {
-
public:
NonConvertingStream()
: composable_stream_(this) {
}
- NonConvertingStream(DataStream *other)
+ explicit NonConvertingStream(DataStream *other)
Review comment:
This breaks API
##########
File path: libminifi/include/controllers/SSLContextService.h
##########
@@ -44,14 +46,12 @@ namespace controllers {
class SSLContext {
public:
#ifdef OPENSSL_SUPPORT
- SSLContext(SSL_CTX *context)
+ explicit SSLContext(SSL_CTX *context)
: context_(context) {
-
}
#else
- SSLContext(void *context) {
-
- }
+explicit SSLContext(void *context) {
Review comment:
This breaks API
##########
File path: libminifi/include/core/ClassLoader.h
##########
@@ -154,32 +151,27 @@ class ObjectFactory {
virtual std::unique_ptr<ObjectFactory> assign(const std::string &class_name)
= 0;
private:
-
std::string group_;
-
};
/**
* Factory that is used as an interface for
* creating processors from shared objects.
*/
template<class T>
class DefautObjectFactory : public ObjectFactory {
-
public:
-
DefautObjectFactory() {
className = core::getClassName<T>();
}
- DefautObjectFactory(const std::string &group_name)
+ explicit DefautObjectFactory(const std::string &group_name)
Review comment:
This breaks API, but I think is unlikely to be used outside of libminifi
##########
File path: libminifi/include/core/Core.h
##########
@@ -56,12 +57,13 @@
#endif
#ifdef _WIN32
-#ifndef WIN32_LEAN_AND_MEAN
+#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN 1
#endif
// can't include cxxabi
#else
#include <cxxabi.h>
+
Review comment:
I don't think this empty line is necessary
##########
File path: libminifi/include/controllers/SSLContextService.h
##########
@@ -44,14 +46,12 @@ namespace controllers {
class SSLContext {
public:
#ifdef OPENSSL_SUPPORT
- SSLContext(SSL_CTX *context)
+ explicit SSLContext(SSL_CTX *context)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/MetricsBase.h
##########
@@ -93,7 +91,7 @@ class DeviceInformation : public ResponseNode {
DeviceInformation(const std::string& name, utils::Identifier & uuid)
: ResponseNode(name, uuid) {
}
- DeviceInformation(const std::string& name)
+ explicit DeviceInformation(const std::string& name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/SchedulingNodes.h
##########
@@ -36,7 +39,7 @@ class SchedulingDefaults : public DeviceInformation {
: DeviceInformation(name, uuid) {
}
- SchedulingDefaults(const std::string &name)
+ explicit SchedulingDefaults(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -282,24 +278,24 @@ class DataSizeValidator : public PropertyValidator {
class PortValidator : public LongValidator {
public:
- PortValidator(const std::string &name)
+ explicit PortValidator(const std::string &name)
: LongValidator(name, 1, 65535) {
}
~PortValidator() override = default;
};
-//Use only for specifying listen ports, where 0 means a randomly chosen one!
+// Use only for specifying listen ports, where 0 means a randomly chosen one!
class ListenPortValidator : public LongValidator {
-public:
- ListenPortValidator(const std::string &name)
+ public:
+ explicit ListenPortValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/SerializableComponent.h
##########
@@ -32,21 +35,16 @@ namespace core {
* Represents a component that is serializable and an extension point of core
Component
*/
class SerializableComponent : public core::Connectable, public
minifi::io::Serializable {
-
public:
-
- SerializableComponent(const std::string name)
+ explicit SerializableComponent(const std::string name)
Review comment:
This may break API for the same reason as `Processor::Processor`
##########
File path: libminifi/include/core/state/nodes/SystemMetrics.h
##########
@@ -94,25 +99,24 @@ class SystemInformation : public DeviceInformation {
arch.value = std::string(buf.machine);
}
- systemInfo.children.push_back(arch);
- serialized.push_back(systemInfo);
+ systemInfo.children.push_back(arch);
+ serialized.push_back(systemInfo);
Review comment:
The indentation is still incorrect
##########
File path: libminifi/include/core/state/nodes/TreeUpdateListener.h
##########
@@ -35,9 +37,8 @@ namespace response {
*/
class MetricsUpdate : public Update {
public:
- MetricsUpdate(UpdateStatus status)
+ explicit MetricsUpdate(UpdateStatus status)
Review comment:
This breaks API
##########
File path: libminifi/include/io/CRCStream.h
##########
@@ -166,7 +173,7 @@ class CRCStream : public BaseStream {
template<typename K>
int readBuffer(std::vector<uint8_t>& buf, const K& t) {
buf.resize(sizeof t);
- return readData((uint8_t*) &buf[0], sizeof(t));
+ return readData(reinterpret_cast<uint8_t*>(&buf[0]), sizeof(t));
Review comment:
This should be `buf.data()`.
##########
File path: libminifi/include/core/Processor.h
##########
@@ -15,35 +15,37 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef __PROCESSOR_H__
-#define __PROCESSOR_H__
+#ifndef LIBMINIFI_INCLUDE_CORE_PROCESSOR_H_
+#define LIBMINIFI_INCLUDE_CORE_PROCESSOR_H_
+
+#include <utils/Id.h>
Review comment:
We should change this to use quotation marks and move it down
##########
File path: libminifi/include/core/Processor.h
##########
@@ -100,9 +101,9 @@ class Processor : public Connectable, public
ConfigurableComponent, public std::
// Set Processor Scheduling Period in Nano Second
void setSchedulingPeriodNano(uint64_t period) {
uint64_t minPeriod = MINIMUM_SCHEDULING_NANOS;
- // std::max has some variances on c++11-c++14 and then c++14 onward.
- // to avoid macro conditional checks we can use this simple conditional
expr.
- scheduling_period_nano_ = period > minPeriod ? period : minPeriod;
+ // std::max has some variances on c++11-c++14 and then c++14 onward.
+ // to avoid macro conditional checks we can use this simple conditional
expr.
+ scheduling_period_nano_ = period > minPeriod ? period : minPeriod;
}
Review comment:
Incorrect indentation not fixed here.
##########
File path: libminifi/include/io/BaseStream.h
##########
@@ -40,13 +43,12 @@ namespace io {
* Extensions may be thread safe and thus shareable, but that is up to the
implementation.
*/
class BaseStream : public DataStream, public Serializable {
-
public:
BaseStream()
: composable_stream_(this) {
}
- BaseStream(DataStream *other)
+ explicit BaseStream(DataStream *other)
Review comment:
This breaks API. Probably nobody should rely on it as `BaseStream`
should be abstract (not sure if it actually is or not), but it's better not to
risk these until the next major version.
##########
File path: libminifi/include/core/ProcessSession.h
##########
@@ -49,12 +51,11 @@ class ProcessSession : public ReferenceContainer {
/*!
* Create a new process session
*/
- ProcessSession(std::shared_ptr<ProcessContext> processContext = nullptr)
+ explicit ProcessSession(std::shared_ptr<ProcessContext> processContext =
nullptr)
Review comment:
This breaks API
##########
File path: libminifi/include/io/DataStream.h
##########
@@ -43,10 +43,10 @@ class DataStream {
* Constructor
**/
explicit DataStream(const uint8_t *buf, const uint32_t buflen) {
- writeData((uint8_t*) buf, buflen);
+ writeData(const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(buf)),
buflen);
Review comment:
Why do we need the `reinterpret_cast` to the same type?
##########
File path: libminifi/include/utils/file/FileUtils.h
##########
@@ -696,13 +695,13 @@ class FileUtils {
return {};
}
#endif /* WIN32 */
-};
+}; // NOLINT
Review comment:
Why is this `NOLINT` here?
##########
File path: libminifi/include/io/tls/TLSSocket.h
##########
@@ -56,7 +62,7 @@ class OpenSSLInitializer {
class TLSContext : public SocketContext {
public:
- TLSContext(const std::shared_ptr<Configure> &configure,
std::shared_ptr<minifi::controllers::SSLContextService> ssl_service = nullptr);
+ explicit TLSContext(const std::shared_ptr<Configure> &configure,
std::shared_ptr<minifi::controllers::SSLContextService> ssl_service = nullptr);
Review comment:
This breaks API
##########
File path: libminifi/include/core/ProcessSessionReadCallback.h
##########
@@ -29,24 +32,24 @@ namespace apache {
namespace nifi {
namespace minifi {
namespace core {
- class ProcessSessionReadCallback : public InputStreamCallback {
- public:
- ProcessSessionReadCallback(const std::string &tmpFile, const
std::string &destFile,
- std::shared_ptr<logging::Logger> logger);
- ~ProcessSessionReadCallback();
- virtual int64_t process(std::shared_ptr<io::BaseStream> stream);
- bool commit();
+class ProcessSessionReadCallback : public InputStreamCallback {
+ public:
+ ProcessSessionReadCallback(const std::string &tmpFile, const std::string
&destFile,
+ std::shared_ptr<logging::Logger> logger);
+ ~ProcessSessionReadCallback();
Review comment:
The indentation is still incorrect here. We should use +1 level (2
spaces) for class members and +2 levels for continuation indentation.
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -85,8 +87,7 @@ class ValidationResult {
class PropertyValidator {
public:
-
- PropertyValidator(std::string name)
+ explicit PropertyValidator(std::string name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/Property.h
##########
@@ -648,23 +648,21 @@ class ConstrainedProperty : public
std::enable_shared_from_this<ConstrainedPrope
return std::move(prop);
}
- ConstrainedProperty(const std::shared_ptr<PropertyBuilder> &builder)
+ explicit ConstrainedProperty(const std::shared_ptr<PropertyBuilder> &builder)
Review comment:
This breaks API
##########
File path: libminifi/include/core/ProcessContextBuilder.h
##########
@@ -57,7 +58,7 @@ class ProcessContextBuilder : public core::CoreComponent,
public std::enable_sha
public:
ProcessContextBuilder(const std::string &name, minifi::utils::Identifier
&uuid);
- ProcessContextBuilder(const std::string &name);
+ explicit ProcessContextBuilder(const std::string &name);
Review comment:
This breaks API
##########
File path: libminifi/include/core/Processor.h
##########
@@ -68,7 +69,7 @@ class Processor : public Connectable, public
ConfigurableComponent, public std::
Processor(std::string name, utils::Identifier &uuid);
- Processor(std::string name);
+ explicit Processor(std::string name);
Review comment:
This may break API. I don't really understand why this class is not
abstract, but if in some arcane way someone depended on the implicit
construction of a `Processor` object (not an actual processor impl) by name,
then this may break that code.
I think the above is unlikely but this API is important for extension
developers, so we should be extremely careful about breaking it in minor
versions. I suggest keeping it non-`explicit` for now and possibly making the
class abstract in the next major version.
##########
File path: libminifi/include/sitetosite/RawSocketProtocol.h
##########
@@ -52,21 +57,21 @@ namespace sitetosite {
*/
typedef struct Site2SitePeerStatus {
std::string host_;
- int port_;bool isSecure_;
+ int port_;
+ bool isSecure_;
} Site2SitePeerStatus;
// RawSiteToSiteClient Class
class RawSiteToSiteClient : public sitetosite::SiteToSiteClient {
public:
-
// HandShakeProperty Str
static const char *HandShakePropertyStr[MAX_HANDSHAKE_PROPERTY];
// Constructor
/*!
* Create a new control protocol
*/
- RawSiteToSiteClient(std::unique_ptr<SiteToSitePeer> peer)
+ explicit RawSiteToSiteClient(std::unique_ptr<SiteToSitePeer> peer)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -68,13 +70,13 @@ class ValidationResult {
std::string input_;
friend class ValidationResult;
};
- private:
+ private:
bool valid_;
std::string subject_;
std::string input_;
- ValidationResult(const Builder &builder)
+ explicit ValidationResult(const Builder &builder)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -132,12 +132,11 @@ class AlwaysValid : public PropertyValidator {
ValidationResult validate(const std::string &subject, const std::string
&input) const override {
return
ValidationResult::Builder::createBuilder().withSubject(subject).withInput(input).isValid(always_valid_).build();
}
-
};
class BooleanValidator : public PropertyValidator {
public:
- BooleanValidator(const std::string &name)
+ explicit BooleanValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/utils/BackTrace.h
##########
@@ -46,7 +48,7 @@ class BackTrace {
public:
BackTrace() = default;
- BackTrace(std::string name)
+ explicit BackTrace(std::string name)
Review comment:
This breaks API
##########
File path: libminifi/include/utils/HTTPClient.h
##########
@@ -138,7 +143,7 @@ struct HTTPHeaderResponse {
size_t separator_pos = header_line.find(':');
if (separator_pos == std::string::npos) {
if (!last_key.empty() && (header_line[0] == ' ' || header_line[0] ==
'\t')) {
- /* This is a "folded header", which is deprecated
(https://www.ietf.org/rfc/rfc7230.txt) but here we are */
+ // This is a "folded header", which is deprecated(https: //
www.ietf.org/rfc/rfc7230.txt) but here we are
Review comment:
The URL was incorrectly "fixed"
##########
File path: libminifi/include/utils/HTTPClient.h
##########
@@ -77,9 +82,9 @@ enum class SSLVersion : uint8_t {
struct HTTPHeaderResponse {
public:
- HTTPHeaderResponse(int max)
- : max_tokens_(max)
- , parsed(false) {
+ explicit HTTPHeaderResponse(int max)
+ : max_tokens_(max)
+ , parsed(false) {
Review comment:
1. `explicit` breaks API.
2. The indentation is still incorrect.
https://google.github.io/styleguide/cppguide.html#Constructor_Initializer_Lists
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -265,7 +261,7 @@ class UnsignedLongValidator : public PropertyValidator {
class DataSizeValidator : public PropertyValidator {
public:
- DataSizeValidator(const std::string &name)
+ explicit DataSizeValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValue.h
##########
@@ -137,7 +141,7 @@ class PropertyValue : public state::response::ValueNode {
* createValue
*/
template<typename T>
- auto operator=(const T ref) -> typename std::enable_if<std::is_same<T,
std::string>::value,PropertyValue&>::type {
+ auto operator=(const T ref) -> typename std::enable_if<std::is_same<T,
std::string>::value, PropertyValue&>::type {
Review comment:
Not your issue, but wtf is this SFINAE check :D
##########
File path: libminifi/include/utils/HTTPClient.h
##########
@@ -176,12 +179,10 @@ class HTTPRequestResponse {
HTTPRequestResponse(const HTTPRequestResponse &other)
: max_queue(other.max_queue) {
-
}
- HTTPRequestResponse(size_t max)
+ explicit HTTPRequestResponse(size_t max)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -157,7 +156,7 @@ class BooleanValidator : public PropertyValidator {
class IntegerValidator : public PropertyValidator {
public:
- IntegerValidator(const std::string &name)
+ explicit IntegerValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -282,24 +278,24 @@ class DataSizeValidator : public PropertyValidator {
class PortValidator : public LongValidator {
public:
- PortValidator(const std::string &name)
+ explicit PortValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/utils/Id.h
##########
@@ -110,7 +109,7 @@ typedef uint8_t UUID_FIELD[16];
class Identifier : public IdentifierBase<UUID_FIELD, std::string> {
public:
- Identifier(UUID_FIELD u);
+ explicit Identifier(UUID_FIELD u);
Review comment:
This breaks API
##########
File path: libminifi/include/utils/Id.h
##########
@@ -52,8 +53,7 @@ namespace utils {
template<typename T, typename C>
class IdentifierBase {
public:
-
- IdentifierBase(T myid) {
+ explicit IdentifierBase(T myid) {
Review comment:
This breaks API
##########
File path: libminifi/include/utils/file/FileManager.h
##########
@@ -72,12 +76,12 @@ class FileManager {
unique_files_.push_back(file_name);
return file_name;
} else {
- std::string tmpDir = "/tmp";
- #ifdef WIN32
- TCHAR lpTempPathBuffer[MAX_PATH];
- GetTempPath(MAX_PATH, lpTempPathBuffer);
- tmpDir = lpTempPathBuffer;
- #endif
+ std::string tmpDir = "/tmp";
+ #ifdef WIN32
+ TCHAR lpTempPathBuffer[MAX_PATH];
+ GetTempPath(MAX_PATH, lpTempPathBuffer);
+ tmpDir = lpTempPathBuffer;
+ #endif
Review comment:
The indentation is still incorrect. Your editor might have got confused
because of the preprocessor checks
##########
File path: libminifi/include/utils/Id.h
##########
@@ -119,7 +118,7 @@ class Identifier : public IdentifierBase<UUID_FIELD,
std::string> {
* I believe these exist to make windows builds happy -- need more testing
* to ensure this doesn't cause any issues.
*/
- Identifier(const IdentifierBase &other);
+ explicit Identifier(const IdentifierBase &other);
Review comment:
This implicit conversion looks intentional to me.
##########
File path: libminifi/include/utils/MinifiConcurrentQueue.h
##########
@@ -87,8 +88,8 @@ class ConcurrentQueue {
}
private:
- ConcurrentQueue(ConcurrentQueue&& other, std::lock_guard<std::mutex>&)
- : queue_( std::move(other.queue_) ) {}
+ ConcurrentQueue(ConcurrentQueue&& other, std::lock_guard<std::mutex>&)
+ : queue_(std::move(other.queue_) ) {}
Review comment:
Spacing inside the paren is inconsistent. I don't have a strong opinion
about whether to use spaces inside initializers, but I want them consistent.
The google style guide suggests no spaces inside.
##########
File path: libminifi/include/core/PropertyValidation.h
##########
@@ -282,24 +278,24 @@ class DataSizeValidator : public PropertyValidator {
class PortValidator : public LongValidator {
public:
- PortValidator(const std::string &name)
+ explicit PortValidator(const std::string &name)
: LongValidator(name, 1, 65535) {
}
~PortValidator() override = default;
};
-//Use only for specifying listen ports, where 0 means a randomly chosen one!
+// Use only for specifying listen ports, where 0 means a randomly chosen one!
class ListenPortValidator : public LongValidator {
-public:
- ListenPortValidator(const std::string &name)
+ public:
+ explicit ListenPortValidator(const std::string &name)
: LongValidator(name, 0, 65535) {
}
~ListenPortValidator() override = default;
};
class TimePeriodValidator : public PropertyValidator {
public:
- TimePeriodValidator(const std::string &name)
+ explicit TimePeriodValidator(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/Resource.h
##########
@@ -1,68 +1,70 @@
-/**
- *
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#ifndef LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
-#define LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
-
-#ifndef WIN32_LEAN_AND_MEAN
-#define WIN32_LEAN_AND_MEAN 1
-#endif
-
-#include "ClassLoader.h"
-#include "agent/agent_docs.h"
-
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace core {
-
-#define MKSOC(x) #x
-#define MAKESTRING(x) MKSOC(x)
-
-template<class T>
-class StaticClassType {
- public:
-
- StaticClassType(const std::string &name, const std::string &description =
"") {
- // Notify when the static member is created
- if (!description.empty()){
- minifi::AgentDocs::putDescription(name,description);
- }
-#ifdef MODULE_NAME
- ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new
DefautObjectFactory<T>(MAKESTRING(MODULE_NAME))));
-#else
- ClassLoader::getDefaultClassLoader().registerClass(name,
std::unique_ptr<ObjectFactory>(new DefautObjectFactory<T>("minifi-system")));
-#endif
- }
-};
-
-#define REGISTER_RESOURCE(CLASSNAME,DESC) \
- static core::StaticClassType<CLASSNAME> \
- CLASSNAME##_registrar( #CLASSNAME, DESC );
-
-#define REGISTER_RESOURCE_AS(CLASSNAME,NAME) \
- static core::StaticClassType<CLASSNAME> \
- CLASSNAME##_registrar( #NAME );
-
-}/* namespace core */
-} /* namespace minifi */
-} /* namespace nifi */
-} /* namespace apache */
-} /* namespace org */
-
-#endif /* LIBMINIFI_INCLUDE_CORE_RESOURCE_H_ */
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
+#define LIBMINIFI_INCLUDE_CORE_RESOURCE_H_
+
+#include <memory>
+#include <string>
+
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN 1
+#endif
+
+#include "ClassLoader.h"
+#include "agent/agent_docs.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+#define MKSOC(x) #x
+#define MAKESTRING(x) MKSOC(x)
+
+template<class T>
+class StaticClassType {
+ public:
+ explicit StaticClassType(const std::string &name, const std::string
&description = "") {
Review comment:
This breaks API
##########
File path: libminifi/include/core/logging/LoggerConfiguration.h
##########
@@ -54,8 +56,7 @@ struct LoggerNamespace {
children(std::map<std::string, std::shared_ptr<LoggerNamespace>>()) {
}
};
-}
-;
+}; // namespace internal
Review comment:
There should be no semicolon after a namespace ending brace.
##########
File path: libminifi/include/utils/file/FileManager.h
##########
@@ -92,12 +96,12 @@ class FileManager {
#ifdef BOOST_VERSION
return boost::filesystem::unique_path().native();
#else
- std::string tmpDir = "/tmp";
- #ifdef WIN32
- TCHAR lpTempPathBuffer[MAX_PATH];
- GetTempPath(MAX_PATH, lpTempPathBuffer);
- tmpDir = lpTempPathBuffer;
- #endif
+ std::string tmpDir = "/tmp";
+ #ifdef WIN32
+ TCHAR lpTempPathBuffer[MAX_PATH];
+ GetTempPath(MAX_PATH, lpTempPathBuffer);
+ tmpDir = lpTempPathBuffer;
+ #endif
Review comment:
The indentation is still incorrect.
##########
File path: libminifi/include/core/controller/ControllerServiceProvider.h
##########
@@ -72,7 +74,7 @@ class ControllerServiceProvider : public CoreComponent,
public ConfigurableCompo
* @param id controller service identifier.
* @return shared pointer to the controller service node.
*/
- virtual std::shared_ptr<ControllerServiceNode> createControllerService(const
std::string &type,const std::string &longType, const std::string &id,
+ virtual std::shared_ptr<ControllerServiceNode> createControllerService(const
std::string &type, const std::string &longType, const std::string &id,
bool firstTimeAdded) = 0;
Review comment:
Could you join these lines to fix the incorrect continuation
indentation?
##########
File path: libminifi/include/core/repository/VolatileProvenanceRepository.h
##########
@@ -31,14 +33,11 @@ namespace repository {
* Volatile provenance repository.
*/
class VolatileProvenanceRepository : public VolatileRepository<std::string> {
-
public:
explicit VolatileProvenanceRepository(std::string repo_name = "",
std::string dir = REPOSITORY_DIRECTORY, int64_t maxPartitionMillis =
MAX_REPOSITORY_ENTRY_LIFE_TIME, int64_t maxPartitionBytes =
MAX_REPOSITORY_STORAGE_SIZE,
uint64_t purgePeriod =
REPOSITORY_PURGE_PERIOD)
- :
core::SerializableComponent(repo_name),VolatileRepository(repo_name.length() >
0 ? repo_name : core::getClassName<VolatileRepository>(), "",
maxPartitionMillis, maxPartitionBytes, purgePeriod)
-
- {
+ : core::SerializableComponent(repo_name),
VolatileRepository(repo_name.length() > 0 ? repo_name :
core::getClassName<VolatileRepository>(), "", maxPartitionMillis,
maxPartitionBytes, purgePeriod) { // NOLINT
Review comment:
I think all of our long lines are perfectly fine and they only have
`NOLINT` on them because they are long. I propose disabling the line length
check in the linter and ignoring the related rule from the google c++ style
guide.
##########
File path: libminifi/include/core/state/Value.h
##########
@@ -491,7 +486,7 @@ struct SerializedResponseNode {
bool collapsible;
std::vector<SerializedResponseNode> children;
- SerializedResponseNode(bool collapsible = true)
+ explicit SerializedResponseNode(bool collapsible = true)
Review comment:
This breaks API
##########
File path: libminifi/include/core/TypedValues.h
##########
@@ -154,10 +159,10 @@ class DataSizeValue : public TransformableValue, public
state::response::UInt64V
: state::response::UInt64Value(value) {
}
-
+
// Convert String to Integer
template<typename T, typename std::enable_if<
- std::is_integral<T>::value>::type* = nullptr>
+ std::is_integral<T>::value>::type* = nullptr>
Review comment:
The correct indentation level is +2 for continuation indentations. I
would fit this on a single line tbh.
https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions
##########
File path: libminifi/include/core/controller/ControllerService.h
##########
@@ -146,12 +146,11 @@ class ControllerService : public ConfigurableComponent,
public Connectable {
return false;
}
- void setLinkedControllerServices( const
std::vector<std::shared_ptr<controller::ControllerService> > &services ){
+ void setLinkedControllerServices(const
std::vector<std::shared_ptr<controller::ControllerService> > &services ) {
Review comment:
Not a proper fix: there are still redundant spaces at the end
##########
File path: libminifi/include/core/repository/FileSystemRepository.h
##########
@@ -34,13 +37,11 @@ namespace repository {
*/
class FileSystemRepository : public core::ContentRepository, public
core::CoreComponent {
public:
- FileSystemRepository(std::string name = getClassName<FileSystemRepository>())
+ explicit FileSystemRepository(std::string name =
getClassName<FileSystemRepository>())
Review comment:
This breaks API
##########
File path: libminifi/include/core/repository/AtomicRepoEntries.h
##########
@@ -76,8 +78,7 @@ noexcept : key_(std::move(other.key_)),
comparator_(std::move(other.comparator_)) {
}
- ~RepoValue()
- {
+ ~RepoValue() {
}
Review comment:
Could be `~RepoValue() = default;`
##########
File path: libminifi/include/core/state/nodes/MetricsBase.h
##########
@@ -103,7 +101,7 @@ class DeviceInformation : public ResponseNode {
*/
class ObjectNode : public ResponseNode {
public:
- ObjectNode(std::string name, utils::Identifier uuid = utils::Identifier())
+ explicit ObjectNode(std::string name, utils::Identifier uuid =
utils::Identifier())
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/AgentInformation.h
##########
@@ -311,7 +309,7 @@ class ExternalManifest : public ComponentManifest {
: ComponentManifest(name, uuid) {
}
- ExternalManifest(const std::string &name)
+ explicit ExternalManifest(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/AgentInformation.h
##########
@@ -334,7 +332,7 @@ class Bundles : public DeviceInformation {
setArray(true);
}
- Bundles(const std::string &name)
+ explicit Bundles(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/UpdateController.h
##########
@@ -69,28 +71,23 @@ class UpdateStatus {
class Update {
public:
-
Update()
: status_(UpdateStatus(UpdateState::INITIATE, 0)) {
}
- Update(UpdateStatus status)
+ explicit Update(UpdateStatus status)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/AgentInformation.h
##########
@@ -400,21 +398,18 @@ class Bundles : public DeviceInformation {
return serialized;
}
-
};
/**
* Justification and Purpose: Provides available extensions for the agent
information block.
*/
class AgentStatus : public StateMonitorNode {
public:
-
AgentStatus(std::string name, utils::Identifier & uuid)
: StateMonitorNode(name, uuid) {
-
}
- AgentStatus(const std::string &name)
+ explicit AgentStatus(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/UpdateController.h
##########
@@ -46,7 +48,7 @@ enum class UpdateState {
*/
class UpdateStatus {
public:
- UpdateStatus(UpdateState state, int16_t reason = 0);
+ explicit UpdateStatus(UpdateState state, int16_t reason = 0);
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/FlowInformation.h
##########
@@ -118,21 +126,20 @@ class FlowVersion : public DeviceInformation {
identifier = std::move(fv.identifier);
return *this;
}
- protected:
+ protected:
mutable std::mutex guard;
std::shared_ptr<FlowIdentifier> identifier;
};
class FlowMonitor : public StateMonitorNode {
public:
-
FlowMonitor(const std::string &name, utils::Identifier &uuid)
: StateMonitorNode(name, uuid) {
}
- FlowMonitor(const std::string &name)
+ explicit FlowMonitor(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/MetricsBase.h
##########
@@ -43,7 +44,7 @@ class ResponseNode : public core::Connectable {
is_array_(false) {
}
- ResponseNode(const std::string& name)
+ explicit ResponseNode(const std::string& name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/SystemMetrics.h
##########
@@ -94,25 +99,24 @@ class SystemInformation : public DeviceInformation {
arch.value = std::string(buf.machine);
}
- systemInfo.children.push_back(arch);
- serialized.push_back(systemInfo);
+ systemInfo.children.push_back(arch);
+ serialized.push_back(systemInfo);
#endif
serialized.push_back(identifier);
-
+
Review comment:
This line is empty and should be removed, because we shouldn't have more
than 1 consecutive empty line in a function body.
##########
File path: libminifi/include/core/state/nodes/DeviceInformation.h
##########
@@ -341,7 +341,7 @@ class DeviceInfoNode : public DeviceInformation {
device_id_ = device.device_id_;
}
- DeviceInfoNode(const std::string &name)
+ explicit DeviceInfoNode(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/AgentInformation.h
##########
@@ -553,12 +545,11 @@ class AgentMonitor {
*/
class AgentManifest : public DeviceInformation {
public:
-
AgentManifest(const std::string& name, utils::Identifier & uuid)
: DeviceInformation(name, uuid) {
}
- AgentManifest(const std::string &name)
+ explicit AgentManifest(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/DeviceInformation.h
##########
@@ -264,8 +265,8 @@ class Device {
char mac[32];
memcpy(mac, LLADDR(sdl), sdl->sdl_alen);
char mac_add[13];
- snprintf(mac_add, 13, "%02X%02X%02X%02X%02X%02X", mac[0], mac[1],
mac[2], mac[3], mac[4], mac[5]);
- ///macs += mac_add;
+ snprintf(mac_add, 13, "%02X%02X%02X%02X%02X%02X", mac[0], mac[1],
mac[2], mac[3], mac[4], mac[5]); // NOLINT
+ // /macs += mac_add;
Review comment:
I think we should remove the third slash
##########
File path: libminifi/include/core/state/nodes/FlowInformation.h
##########
@@ -156,12 +162,11 @@ class FlowMonitor : public StateMonitorNode {
*/
class FlowInformation : public FlowMonitor {
public:
-
FlowInformation(const std::string &name, utils::Identifier &uuid)
: FlowMonitor(name, uuid) {
}
- FlowInformation(const std::string &name)
+ explicit FlowInformation(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/io/ClientSocket.h
##########
@@ -74,7 +75,7 @@ bool valid_socket(SocketDescriptor) noexcept;
*/
class SocketContext {
public:
- SocketContext(const std::shared_ptr<Configure> &configure) {
+ explicit SocketContext(const std::shared_ptr<Configure> &configure) {
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/ProcessMetrics.h
##########
@@ -42,12 +48,11 @@ namespace response {
*/
class ProcessMetrics : public ResponseNode {
public:
-
ProcessMetrics(const std::string &name, utils::Identifier &uuid)
: ResponseNode(name, uuid) {
}
- ProcessMetrics(const std::string &name)
+ explicit ProcessMetrics(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/properties/Properties.h
##########
@@ -31,13 +33,12 @@ namespace minifi {
class Properties {
public:
- Properties(const std::string& name = "");
+ explicit Properties(const std::string& name = "");
Review comment:
This breaks API
##########
File path: libminifi/include/io/StreamFactory.h
##########
@@ -88,15 +91,15 @@ class StreamFactory {
}
protected:
- StreamFactory(const std::shared_ptr<Configure> &configure);
+ explicit StreamFactory(const std::shared_ptr<Configure> &configure);
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/QueueMetrics.h
##########
@@ -37,20 +41,19 @@ namespace response {
*/
class QueueMetrics : public ResponseNode {
public:
-
QueueMetrics(const std::string &name, utils::Identifier & uuid)
: ResponseNode(name, uuid) {
}
- QueueMetrics(const std::string &name)
+ explicit QueueMetrics(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/RepositoryMetrics.h
##########
@@ -37,12 +41,11 @@ namespace response {
*/
class RepositoryMetrics : public ResponseNode {
public:
-
RepositoryMetrics(const std::string &name, utils::Identifier &uuid)
: ResponseNode(name, uuid) {
}
- RepositoryMetrics(const std::string &name)
+ explicit RepositoryMetrics(const std::string &name)
Review comment:
This breaks API
##########
File path: libminifi/include/core/state/nodes/StateMonitor.h
##########
@@ -41,14 +44,12 @@ namespace response {
class StateMonitorNode : public DeviceInformation {
public:
-
StateMonitorNode(std::string name, utils::Identifier &uuid)
: DeviceInformation(name, uuid),
monitor_(nullptr) {
-
}
- StateMonitorNode(const std::string &name)
+ explicit StateMonitorNode(const std::string &name)
Review comment:
This breaks API
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
