wjones127 commented on code in PR #34889:
URL: https://github.com/apache/arrow/pull/34889#discussion_r1161837285
##########
cpp/src/parquet/metadata.h:
##########
@@ -508,26 +508,29 @@ class PARQUET_EXPORT RowGroupMetaDataBuilder {
class PARQUET_EXPORT FileMetaDataBuilder {
public:
- // API convenience to get a MetaData reader
+ // API convenience to get a MetaData builder
+ PARQUET_DEPRECATED("Deprecated. Use overload without KeyValueMetadata
instead.")
Review Comment:
It's nice to say when it was deprecated.
```suggestion
PARQUET_DEPRECATED("Deprecated in 12.0.0. Use overload without
KeyValueMetadata instead.")
```
##########
cpp/src/parquet/metadata.h:
##########
@@ -510,24 +510,23 @@ class PARQUET_EXPORT FileMetaDataBuilder {
public:
// API convenience to get a MetaData reader
static std::unique_ptr<FileMetaDataBuilder> Make(
- const SchemaDescriptor* schema, std::shared_ptr<WriterProperties> props,
- std::shared_ptr<const KeyValueMetadata> key_value_metadata = NULLPTR);
+ const SchemaDescriptor* schema, std::shared_ptr<WriterProperties> props);
Review Comment:
Ignoring the parameter seems like an even worse breaking change than
removing it. Could we simply make this backwards compatible? Either
`key_value_metadata` in `Finish` in override, or it should merge with the one
provided in the constructor.
--
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]
