Skip to content

Conversation

@HJLebbink
Copy link
Member

@HJLebbink HJLebbink commented Sep 10, 2025

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

@HJLebbink HJLebbink added the cleanup-rewrite Used in release doc generation label Sep 10, 2025
@HJLebbink HJLebbink self-assigned this Sep 10, 2025
Copy link
Contributor

Copilot AI left a 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 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

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.

@HJLebbink HJLebbink force-pushed the cleanup-defaults branch 4 times, most recently from 303c43e to f7396a9 Compare September 10, 2025 15:48
@HJLebbink HJLebbink marked this pull request as ready for review September 10, 2025 15:55
@HJLebbink HJLebbink force-pushed the cleanup-defaults branch 2 times, most recently from 119def6 to 55acda0 Compare September 17, 2025 18:43
Copy link
Contributor

@twuebi twuebi left a 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!

Copy link
Contributor

Copilot AI left a 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.

Copy link
Contributor

Copilot AI left a 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.

@HJLebbink HJLebbink merged commit e16d6d4 into minio:master Oct 16, 2025
6 checks passed
@HJLebbink HJLebbink deleted the cleanup-defaults branch October 16, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup-rewrite Used in release doc generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants