Skip to content

ToTokens refactor #754

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

Merged
merged 10 commits into from
May 19, 2022
Merged

ToTokens refactor #754

merged 10 commits into from
May 19, 2022

Conversation

cataggar
Copy link
Member

This is a good milestone for the ToTokens refactor. Several new types are created that implement quote::ToTokens. This assists in testing the transform from OpenAPI types. Any types that implement ToTokens can be used directing in quote! macro. This makes it possible to incrementally refactor out the types.

@cataggar cataggar requested a review from bmc-msft May 18, 2022 22:25
Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I have some small suggestions for improvement, but nothing blocking. 🎉

@@ -1,3 +1,4 @@
#![allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

}
}

enum TypeCode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all these "code" types, I'd really appreciate a doc comment that points out what they are:

/// A struct field
///
/// ```rust
/// struct My Struct {
///   my_field: u32 // <-- refers to this
/// }
/// ```
struct FieldCode {

@cataggar cataggar merged commit b4c4785 into Azure:main May 19, 2022
@cataggar cataggar deleted the to_tokens2 branch May 19, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants