zhjwpku commented on code in PR #936:
URL: https://github.com/apache/iceberg-cpp/pull/936#discussion_r4229434349
##########
src/iceberg/update/snapshot_update.h:
##########
@@ -66,96 +77,124 @@ class ICEBERG_EXPORT SnapshotUpdate : public PendingUpdate
{
///
/// \param reporter The metrics reporter to use.
/// \return Reference to this for method chaining.
+#if ICEBERG_CXX20_COMPAT
Review Comment:
Could we use the C++20 approach instead of the macro with C++23 explicit
object parameter? cc @wgtmac
##########
src/iceberg/logging/log_level.h:
##########
@@ -66,7 +67,7 @@ constexpr std::string_view ToString(LogLevel level) noexcept {
case LogLevel::kOff:
return "off";
}
- std::unreachable();
+ ::iceberg::unreachable();
Review Comment:
Moving the `ToString`s from `.h` to `.cc` would let us drop
`iceberg/compat/unreachable.h` and use `std::unreachable()`, but I'm fine with
the current approach.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
