Skip to content
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

Feedback for Generated Code #53

Closed
davidbarsky opened this issue Nov 24, 2020 · 1 comment
Closed

Feedback for Generated Code #53

davidbarsky opened this issue Nov 24, 2020 · 1 comment

Comments

@davidbarsky
Copy link
Contributor

As requested by @rcoh; here are few things I've noticed from reading the documentation.

  • dynamo::model::FailureException is the only type in the model module that represents an error. I think this happened because the underlying Smithy definition might be incorrect.
    • Generally speaking, I don't think that "Exception" shouldn't appear in any Rust code
  • The operation module is noisy from the builder module [appendix one]. It also hides the actual input operations below the documentation's fold. Unfortunately, applying #[doc(hidden)] to a module applies to all nested modules and exports, which means that the builder options are hidden: rustdoc: doc(hidden) also hides re-exports rust-lang/rust#59368
  • When documenting input structs, consider using the operation's documentation instead of the input object's documentation [appendix two]. Input structs are the customer's entrypoint to the service, not the operation itself. With a sans-io interface, there's no place to document operations, which often serve as the why of a service's operation.
  • The separate impl blocks for inputs without different generic bounds feel off to me [appendix three]. I'd unify the two.
    • I'm not sure that an assemble associated function is necessary for non-streaming operations, but I believe this relates to the (somewhat) open design question around (de)serialization.
  • Intra-doc links don't resolve to a URL. You might need to define a [docs-url] in the root of each crate: [WIP] Add cargo-deadlinks to CI tokio-rs/tracing#1078 (comment).
    • Consider using the tokio-rs/tracing repo as an example for documentation configuration. Members of the rustdoc team have use tokio-rs/tracing as a testbed for unstable features.

Appendix

Appendix One

Screen Shot 2020-11-24 at 10 51 36 AM

Appendix Two

Screen Shot 2020-11-24 at 10 54 58 AM

Appendix Three

Screen Shot 2020-11-24 at 11 06 05 AM

This was referenced Nov 28, 2020
@jdisanti
Copy link
Collaborator

We should take another pass on this feedback before GA. Adding it to the milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants