-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(rust): Improves documentation for GCP and adds setters to ScanArgsParquet #15759
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #15759 will not alter performanceComparing Summary
|
ea28d9d
to
ef04687
Compare
ef04687
to
a41552b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15759 +/- ##
========================================
Coverage 81.35% 81.35%
========================================
Files 1379 1379
Lines 176603 176879 +276
Branches 2544 2543 -1
========================================
+ Hits 143677 143904 +227
- Misses 32443 32493 +50
+ Partials 483 482 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see the point of setter methods on a struct with public attributes?
Hey! Thanks for taking the time to review this PR. I would say the main reason is ergonomics. For nested functions it's easier than declaring a new mutable variable and modifying it or using the ..Default::default() syntax. I see other important structs like the CsvReader have setters which confused me for a while thinking that ScanArgsParquet would also. Perhaps it's worth thinking of a unified setter/getter language for the different crates? |
This is a very small PR that makes some small quality of life improvements.
I added some setters for ScanArgsParquet to make it easier to built these options.
I added some documentation for CloudOptions
with_gcp
to make it more clear on how to use.