szaszm commented on code in PR #2209:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2209#discussion_r3527731976
##########
core-framework/common/src/utils/Id.cpp:
##########
@@ -43,6 +43,18 @@ bool Identifier::isNil() const {
return *this == Identifier{};
}
+Identifier& Identifier::operator++() {
+ for (int byte_idx = 15; byte_idx >= 0 && ++data_[byte_idx] == 0; --byte_idx)
{}
Review Comment:
This is a bit too dense and clever for my liking, I'd separate the iteration
from the rest of the increment + carry logic.
```suggestion
// increment with carry
for (int byte_idx = 15; byte_idx >= 0; --byte_idx) {
++data_[byte_idx];
if (data_[byte_idx] != 0) {
break;
}
}
```
##########
minifi-api/common/include/minifi-cpp/utils/Id.h:
##########
@@ -46,6 +46,9 @@ class Identifier {
return !isNil();
}
+ Identifier& operator++();
+ Identifier operator++(int);
+
Review Comment:
why do we need these UUID increments? I believe the Identified class is not
meant to represent integer auto-increment keys.
##########
libminifi/include/SchedulingAgent.h:
##########
@@ -50,7 +50,7 @@ namespace org::apache::nifi::minifi {
class SchedulingAgent {
public:
- SchedulingAgent(const
gsl::not_null<core::controller::ControllerServiceProvider*>
controller_service_provider, std::shared_ptr<core::Repository> repo,
std::shared_ptr<core::Repository> flow_repo,
+ SchedulingAgent(const
gsl::not_null<core::controller::ControllerServiceProvider*>
controller_service_provider, std::shared_ptr<provenance::ProvenanceRepository>
repo, std::shared_ptr<core::Repository> flow_repo,
Review Comment:
I believe the "repo" naming of identifiers that have to be provenance repo
was a mistake. I'm glad the type makes it clearer now, but I'd change the
parameter and member names too. `provenance_repo` or `prov_repo` seem like good
options to me.
--
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]
