kou commented on code in PR #44819:
URL: https://github.com/apache/arrow/pull/44819#discussion_r1866861859
##########
cpp/src/parquet/arrow/reader.cc:
##########
@@ -1315,20 +1315,32 @@ Status FileReader::GetRecordBatchReader(const
std::vector<int>& row_group_indice
return Status::OK();
}
-Status FileReader::Make(::arrow::MemoryPool* pool,
- std::unique_ptr<ParquetFileReader> reader,
+Status FileReader::Make(MemoryPool* pool, std::unique_ptr<ParquetFileReader>
reader,
const ArrowReaderProperties& properties,
std::unique_ptr<FileReader>* out) {
*out = std::make_unique<FileReaderImpl>(pool, std::move(reader), properties);
return static_cast<FileReaderImpl*>(out->get())->Init();
Review Comment:
Can we use the `Result` version here?
```suggestion
return Make(pool, std::move(reader), properties)->Value(out);
```
##########
cpp/src/parquet/arrow/reader.h:
##########
@@ -115,17 +115,32 @@ class RowGroupReader;
// arrays
class PARQUET_EXPORT FileReader {
public:
- /// Factory function to create a FileReader from a ParquetFileReader and
properties
+ // Factory function to create a FileReader from a ParquetFileReader and
properties.
+ // \deprecated Deprecated in 19.0.0. Use arrow::Result version instead.
+ ARROW_DEPRECATED("Deprecated in 19.0.0. Use arrow::Result version instead.")
static ::arrow::Status Make(::arrow::MemoryPool* pool,
std::unique_ptr<ParquetFileReader> reader,
const ArrowReaderProperties& properties,
std::unique_ptr<FileReader>* out);
- /// Factory function to create a FileReader from a ParquetFileReader
+ // Factory function to create a FileReader from a ParquetFileReader and
properties
+ // Returns an arrow::Result containing a unique pointer to the FileReader.
Review Comment:
Could you use `\return` Doxygen markup?
https://www.doxygen.nl/manual/commands.html#cmdreturn
##########
cpp/src/parquet/arrow/reader.h:
##########
@@ -115,17 +115,32 @@ class RowGroupReader;
// arrays
class PARQUET_EXPORT FileReader {
public:
- /// Factory function to create a FileReader from a ParquetFileReader and
properties
+ // Factory function to create a FileReader from a ParquetFileReader and
properties.
+ // \deprecated Deprecated in 19.0.0. Use arrow::Result version instead.
Review Comment:
We should use `///` not `//` for Doxygen:
https://www.doxygen.nl/manual/docblocks.html
```suggestion
/// Factory function to create a FileReader from a ParquetFileReader and
properties.
/// \deprecated Deprecated in 19.0.0. Use arrow::Result version instead.
```
--
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]
