Fix Result name collision in parquet_derive
#7548
Conversation
|
Thank you for the contribution @jspaezp This code looks good to me. Is there any chance you can make a test for this (that would fail if we accidentally broke this feature)? Perhaps you could make a small |
|
@alamb thanks for the fast reply! I added a test (added it in parquet_derive_test/src/lib.rs rather than parquet-derive/test/macro_hygene.rs). LMK if this is what you had in mind! |
parquet_derive_test/src/lib.rs
Outdated
|
|
||
| mod aliased_result { | ||
| use parquet_derive::{ParquetRecordReader, ParquetRecordWriter}; | ||
| pub type Result = std::result::Result<(), Box<dyn std::error::Error>>; |
There was a problem hiding this comment.
| pub type Result = std::result::Result<(), Box<dyn std::error::Error>>; | |
| // not an actual result type | |
| pub type Result = (); |
There was a problem hiding this comment.
Did the modification but left the other one commented out so there is a bit more historical context (hope thats ok)
parquet_derive_test/src/lib.rs
Outdated
| } | ||
|
|
||
| impl ARecord { | ||
| pub fn validate(&self) -> Result { |
There was a problem hiding this comment.
| pub fn validate(&self) -> Result { | |
| pub fn validate(&self) -> std::result::Result<(), Box<dyn std::error::Error>> { |
(so it doesn't use our Result shadow)
Which issue does this PR close?
Closes #7547
Rationale for this change
Explained in the issue.
What changes are included in this PR?
Pretty simple change from
Resulttostd::result::Resultto prevent the 'name collision' on the code/text generated from the macro.Are there any user-facing changes?
None that I can think of.
Please let me know if there is anything else you might want/need on my end! and thank you very much for this project.