-
Notifications
You must be signed in to change notification settings - Fork 72
use crate TypedBuilder for builders #184
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
Conversation
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.
Pull Request Overview
This PR migrates from hand-rolled builder structs to the TypedBuilder crate, implementing a standardized builder pattern across the entire S3 client library. This change introduces the .build() method call requirement for all request builders while maintaining the existing API surface.
- Adopts
TypedBuilderfor consistent, type-safe builder patterns across all S3 operations - Updates all client methods to return
TypedBuilder-generated builder types - Adds required
.build()calls to all test files for new builder API
Reviewed Changes
Copilot reviewed 204 out of 204 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| test files | Updated all test method calls to include required .build() step |
| client files | Modified all client methods to return TypedBuilder-generated builders |
| types.rs | Updated S3Request to use TypedBuilder with proper field annotations |
| response files | Updated documentation references from Client to MinioClient |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
303c43e to
f7396a9
Compare
119def6 to
55acda0
Compare
bugfixes WIP doc updated
c855dfe to
2c6f943
Compare
twuebi
left a comment
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.
some nits, mostly good!
2ed425a to
b39feb1
Compare
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.
Pull Request Overview
Copilot reviewed 204 out of 204 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (2)
src/s3/types.rs:1
- Option fields on S3Request are annotated with setter(into), but many call sites pass bare String/Arc values (not Option<>), which do not satisfy Into<Option<>> and will not compile. Either wrap these values in Some(...) at call sites, or change the builder to accept bare values by using setter(strip_option) for Option fields. If you keep accepting Option<_> here (recommended), update all S3Request::builder() usages to call .bucket(Some(...)), .object(Some(...)), and .body(Some(...)).
// MinIO Rust Library for Amazon S3 Compatible Cloud Storage
src/s3/builders/select_object_content.rs:1
- All three builder calls here mismatch field types on S3Request: .bucket(...) and .object(...) need Option (wrap with Some), and .body(...) needs Option<Arc> (wrap with Some). Without these, this will fail to compile.
// MinIO Rust Library for Amazon S3 Compatible Cloud Storage
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
b39feb1 to
39301b0
Compare
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.
Pull Request Overview
Copilot reviewed 204 out of 204 changed files in this pull request and generated 12 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
39301b0 to
991b1cb
Compare
This PR migrates from hand-rolled builder structs to the TypedBuilder crate, implementing a standardized builder pattern across the entire S3 client library. This change introduces the .build() method call requirement for all request builders while maintaining the existing API surface.
Adopts TypedBuilder for consistent, type-safe builder patterns across all S3 operations
Updates all client methods to return TypedBuilder-generated builder types
Adds required .build() calls to all test files for new builder API