evindj commented on code in PR #812:
URL: https://github.com/apache/iceberg-cpp/pull/812#discussion_r3526932866


##########
.github/workflows/test.yml:
##########
@@ -39,11 +40,15 @@ env:
 jobs:
   ubuntu:
     if: ${{ github.event_name != 'pull_request' || 
github.event.pull_request.draft == false }}
-    name: AMD64 Ubuntu 26.04
+    name: AMD64 Ubuntu 26.04 (${{ matrix.cmake_build_type }})
     runs-on: ubuntu-26.04
-    timeout-minutes: 30
+    timeout-minutes: 60

Review Comment:
   Is my understanding correct that the timeout was doubled becuase now for 
ubuntu 2 build configurations are going to be executed?



##########
src/iceberg/json_serde.cc:
##########
@@ -1377,9 +1377,9 @@ Result<std::unique_ptr<TableMetadata>> 
TableMetadataFromJson(const nlohmann::jso
       TimePointMs{std::chrono::milliseconds(last_updated_ms)};
 
   if (json.contains(kRefs)) {
-    ICEBERG_ASSIGN_OR_RAISE(
-        table_metadata->refs,
-        FromJsonMap<std::shared_ptr<SnapshotRef>>(json, kRefs, 
SnapshotRefFromJson));
+    ICEBERG_ASSIGN_OR_RAISE(auto refs, 
FromJsonMap<std::shared_ptr<SnapshotRef>>(
+                                           json, kRefs, SnapshotRefFromJson));
+    table_metadata->refs = std::move(refs);

Review Comment:
   seems like a more readable version of previous version, however it seems 
disconnected from the change discussed in this PR?



-- 
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]

Reply via email to