szaszm commented on code in PR #1377:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1377#discussion_r941415079
##########
libminifi/src/c2/C2Agent.cpp:
##########
@@ -739,7 +739,7 @@ utils::TaskRescheduleInfo C2Agent::produce() {
std::make_move_iterator(payload_batch.end()),
[&] (C2Payload&& payload) {
try {
- C2Payload && response =
protocol_.load()->consumePayload(std::move(payload));
+ C2Payload && response = protocol_.load()->consumePayload(payload);
Review Comment:
Could you change `response` from reference to variable? I find it confusing
whenever code unnecessarily relies on temporary lifetime extension, when it
could just normally store the data on the stack.
##########
libminifi/test/unit/ExpectedTest.cpp:
##########
@@ -52,7 +52,7 @@ TEST_CASE("expected map", "[expected][map]") {
{
const nonstd::expected<int, int> e = 21;
- auto ret = std::move(e) | utils::map(mul2);
+ auto ret = e | utils::map(mul2);
Review Comment:
These are meant to test utilities on rvalues. You can see the same tests
above on lvalues. Please revert these changes, except for the last one.
##########
libminifi/test/unit/IdTests.cpp:
##########
@@ -77,7 +77,7 @@ TEST_CASE("Test Generate Move", "[id]") {
auto generated = generator->generate();
auto str = generated.to_string();
- utils::Identifier moved = std::move(generated);
+ utils::Identifier moved = generated;
Review Comment:
This change is also removing an intentional move.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
