westonpace commented on a change in pull request #10205: URL: https://github.com/apache/arrow/pull/10205#discussion_r626239342
########## File path: cpp/src/arrow/util/thread_pool_test.cc ########## @@ -189,11 +189,7 @@ TEST_P(TestRunSynchronously, SpawnMoreNested) { auto top_level_task = [&](Executor* executor) -> Future<> { auto fut_a = DeferNotOk(executor->Submit([&] { nested_ran++; })); auto fut_b = DeferNotOk(executor->Submit([&] { nested_ran++; })); - return AllComplete({fut_a, fut_b}) - .Then([&](const Result<arrow::detail::Empty>& result) { - nested_ran++; - return result; - }); + return AllComplete({fut_a, fut_b}).Then([&]() { nested_ran++; }); Review comment: Thanks, this is great. `call_traits` doesn't work for function pointers so that caused one call to be slightly less readable (there was already a TODO but I promoted it to a full JIRA issue ARROW-12655. However, I think it's worth it. This caught several places we were accepting `Result` and one in `async_generator.h` that was potentially a bug. -- 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