refactor: suppress warnings #328
Conversation
wgtmac
left a comment
wgtmac
left a comment
There was a problem hiding this comment.
Thanks for working on this! I've left a minor comment. Please also fix the linter errors.
| std::unordered_map<std::string, std::string> defaults; // required | ||
| std::unordered_map<std::string, std::string> overrides; // required | ||
| std::vector<std::string> endpoints; | ||
| std::unordered_map<std::string, std::string> defaults{}; // required |
There was a problem hiding this comment.
We don't need to add {} for everything, at least not for STL containers and std::string. Please try to minimize lines of change like this.
| const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, | ||
| const std::string& location, | ||
| const std::unordered_map<std::string, std::string>& properties) { | ||
| [[maybe_unused]] const TableIdentifier& identifier, |
There was a problem hiding this comment.
I think we can remove the parameters' names instead of adding [[maybe_unused]] to keep the code clear.
| case TypeId::kFixed: { | ||
| auto target_fixed_type = internal::checked_pointer_cast<FixedType>(target_type); | ||
| if (binary_val.size() == target_fixed_type->length()) { | ||
| if (static_cast<int32_t>(binary_val.size()) == target_fixed_type->length()) { |
There was a problem hiding this comment.
In general, we should convert small types to large types. There are some similar codes in this PR that need to be modified.
| } | ||
|
|
||
| std::shared_ptr<Type> CreateListOfList() { | ||
| [[maybe_unused]] std::shared_ptr<Type> CreateListOfList() { |
There was a problem hiding this comment.
@wgtmac Does this method not being used mean we are missing some tests?
| friend bool operator==(const Schema& lhs, const Schema& rhs) { return lhs.Equals(rhs); } | ||
|
|
||
| private: | ||
| using Type::Equals; |
There was a problem hiding this comment.
@wgtmac It seems that we should override the Equals methods in Schema instead of adding Schema::Equals(const Schema& other) const at line 108?
3 participants
Warnings have been suppressed according to the following patterns:
[[maybe_unused]]using base::func.