fgerlits commented on code in PR #1661:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1661#discussion_r1337362047
##########
libminifi/src/core/extension/ExtensionManager.cpp:
##########
@@ -33,7 +33,7 @@ const std::shared_ptr<logging::Logger>
ExtensionManager::logger_ = logging::Logg
ExtensionManager::ExtensionManager() {
modules_.push_back(std::make_unique<Executable>());
- active_module_ = modules_[0].get();
+ active_module_ = modules_[0].get(); //
NOLINT(cppcoreguidelines-prefer-member-initializer)
}
Review Comment:
I'm not sure if this is better, but we could use initializers this way:
```c++
ExtensionManager::ExtensionManager()
: modules_([]{
std::vector<std::unique_ptr<Module>> modules;
modules.push_back(std::make_unique<Executable>());
return modules;
}()),
active_module_(modules_[0].get()) {
}
```
But I'm OK with the NOLINT solution, too.
##########
extensions/librdkafka/PublishKafka.cpp:
##########
@@ -619,7 +622,7 @@ bool PublishKafka::createNewTopic(const
std::shared_ptr<core::ProcessContext> &c
}
// The topic takes ownership of the configuration, we must not free it
- gsl::owner<rd_kafka_topic_t*> topic_reference =
rd_kafka_topic_new(conn_->getConnection(), topic_name.c_str(),
topic_conf_.release());
+ const auto topic_reference =
gsl::owner<rd_kafka_topic_t*>(rd_kafka_topic_new(conn_->getConnection(),
topic_name.c_str(), topic_conf_.release())); //
NOLINT(cppcoreguidelines-owning-memory)
Review Comment:
just out of curiosity, why is the NOLINT comment needed? it looks like we
are correctly assigning to an `owner`
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -92,7 +92,7 @@ void ComponentManifest::serializeClassDescription(const
std::vector<ClassDescrip
child.children.push_back({.name = "validator", .value =
std::string{prop.getValidator().getValidatorName()}});
child.children.push_back({.name = "required", .value =
prop.getRequired()});
child.children.push_back({.name = "expressionLanguageScope", .value =
prop.supportsExpressionLanguage() ? "FLOWFILE_ATTRIBUTES" : "NONE"});
- child.children.push_back({.name = "defaultValue", .value =
prop.getValue()});
+ child.children.push_back({.name = "defaultValue", .value =
prop.getValue()}); // NOLINT(cppcoreguidelines-slicing)
Review Comment:
Not in this PR, but I think we should fix this slicing issue at some point.
The simplest way would be a `toValueNode()` function in `PropertyValue`, but
there may be a nicer solution. Can you create a Jira for this, please?
##########
libminifi/test/unit/FilePatternTests.cpp:
##########
Review Comment:
minor, but all these `REQUIRE`s should be `CHECK`s
##########
extensions/expression-language/Expression.cpp:
##########
@@ -118,21 +118,21 @@ Value resolve_user_id(const std::vector<Value> &args) {
}
Value expr_hostname(const std::vector<Value> &args) {
- char hostname[1024];
+ std::array<char, 1024> hostname{};
hostname[1023] = '\0';
Review Comment:
very minor, but with the `{}`, the array is already filled with 0s, so this
line is not necessary
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
