You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2025/02/04 03:30:54 UTC

[PR] Make field repr explicit about optionality, add exception type [iceberg-cpp]

lidavidm opened a new pull request, #43:
URL: https://github.com/apache/iceberg-cpp/pull/43

   - Fix some clang-tidy lints (`virtual ~Foo() = default` -> `~Foo() override = default`, missing `explicit`)
   - Add `iceberg::IcebergError` instead of `std::runtime_error`
   - Make field repr explicitly put `(optional)`
   - Note current time complexity of field accessors


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Make field repr explicit about optionality, add exception type [iceberg-cpp]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #43:
URL: https://github.com/apache/iceberg-cpp/pull/43


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Make field repr explicit about optionality, add exception type [iceberg-cpp]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #43:
URL: https://github.com/apache/iceberg-cpp/pull/43#issuecomment-2664805968

   @Fokko @Xuanwo Could you help merge this? Thanks!


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Make field repr explicit about optionality, add exception type [iceberg-cpp]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #43:
URL: https://github.com/apache/iceberg-cpp/pull/43#issuecomment-2652797693

   Any more comments here?


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Make field repr explicit about optionality, add exception type [iceberg-cpp]

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #43:
URL: https://github.com/apache/iceberg-cpp/pull/43#discussion_r1940581911


##########
src/iceberg/schema_field.cc:
##########
@@ -52,8 +52,8 @@ const std::shared_ptr<Type>& SchemaField::type() const { return type_; }
 bool SchemaField::optional() const { return optional_; }
 
 std::string SchemaField::ToString() const {
-  return std::format("{} ({}): {}{}", name_, field_id_, *type_,
-                     optional_ ? "" : " (required)");
+  return std::format("{} ({}): {} {}", name_, field_id_, *type_,
+                     optional_ ? "(optional)" : "(required)");

Review Comment:
   ```suggestion
     return std::format("{} ({}): {} ({})", name_, field_id_, *type_,
                        optional_ ? "optional" : "required");
   ```



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org