RFC: Client Crate Organization #1936
Conversation
|
This is very much a work in progress. I put some ideas down on paper, but I'm not entirely happy with it and would love suggestions. There is a TODO in the errors section that especially needs attention. I'm wondering if, instead of what the RFC is currently proposing, we should put all code generated types inside of |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| ``` | ||
|
|
||
| The `AppName` is infrequently set, and should be moved into `crate::config`. | ||
| The `Error` and `ErrorExt` types should be moved into `crate::error` since |
There was a problem hiding this comment.
totally agree about ErrorExt. Depending on where you land on errors, I think keeping Error in the root so you can do without any duplicate error::Error is probably worthwhile.
The other stuff in the root like Endpoint should be moved. PKG_VERSION could be moved into a meta module. It's debatable if Region needs to be re-exported at all now that we have aws-config.
async fn do_a_thing() -> Result<String, aws_sdk_s3::Error> -> { ... }
There was a problem hiding this comment.
Your reason for keeping Error in the top-level makes sense to me.
I think Region should still be re-exported since it's referenced by Config and the config's Builder. It can be moved into crate::config though.
| Empty Modules | ||
| ------------- | ||
|
|
||
| The `lens` and `http_body_checksum` modules have nothing inside them, |
There was a problem hiding this comment.
if lens isn't private that's a bug 🤭
Co-authored-by: Russell Cohen <rcoh@amazon.com>
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
The changes being proposed in this RFC will also impact server crate organization unless we do additional work to keep the server the same as it is now. Adding the |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| The `AppName` is infrequently set, and should be moved into `crate::config`. The `aws-config` crate | ||
| drastically reduces the usefulness of `Credentials`, `Endpoint`, and `Region`, so these can move | ||
| into `crate::config`. `ErrorExt` should be moved into `crate::error`, but `Error` should stay in |
There was a problem hiding this comment.
| The `AppName` is infrequently set, and should be moved into `crate::config`. The `aws-config` crate | |
| drastically reduces the usefulness of `Credentials`, `Endpoint`, and `Region`, so these can move | |
| into `crate::config`. `ErrorExt` should be moved into `crate::error`, but `Error` should stay in | |
| The `AppName` is infrequently set, and should be moved into `crate::config`. We currently encourage users to import `Credentials`, `Endpoint`, and `Region` from the `aws-config` crate, so service crate reexports of these types can move into `crate::config`. `ErrorExt` should be moved into `crate::error`, but `Error` should stay in |
There was a problem hiding this comment.
Looking at this RFC, it's making me wonder again whether we should not also be feature-gating each operation in order to shorten compile times. We could generate the feature flags for all ops and then include them all by default. Users wanting short compile times would disable default features and include only what they use.
|
Decided to move the "combine |
|
The RFC has been updated to organize code generated types by operation. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-rfc-client-crate-org
branch
from
3748002
to
df19d7a
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
This RFC proposes some organizational changes to the generated SDK and client crates to make navigating the documentation easier while also facilitating organized re-export of runtime crate types.
Rendered
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.