Skip to content

Conversation

@ltitanb
Copy link
Collaborator

@ltitanb ltitanb commented Aug 8, 2024

  • Simplify the BuilderApi and BuilderApiState traits
  • Add a working example on how to use the Api in examples/status_api

fix issues from #69

@ltitanb ltitanb requested review from David-Petrov and fbrv August 8, 2024 20:36
@ltitanb ltitanb self-assigned this Aug 8, 2024
Copy link
Contributor

@fbrv fbrv left a comment

Choose a reason for hiding this comment

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

gtg, we should a bit more descriptive with generics name. Ie,

struct PbsState<S> {
  data: S
}

could be renamed as

puct struct PbsState<EData> {
  data: EData,
}

In order to make the code a easier to read in case someone miss bits of documentation

Copy link
Contributor

@David-Petrov David-Petrov left a comment

Choose a reason for hiding this comment

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

Everything looks fine!

Regarding what Fabio said about generic type parameters' names, I'm also a fan of more descriptive (whole-word even) type names, but this can easily become cumbersome, too. For me, right now a satisfactory solution would be to just describe type parameters in the rustdoc of the struct/trait that introduces them.

@ltitanb
Copy link
Collaborator Author

ltitanb commented Aug 9, 2024

Sounds good, will merge as of now and add this note in the open issue we have for internal docs

@ltitanb ltitanb merged commit c6b44a4 into main Aug 9, 2024
@ltitanb ltitanb deleted the ltitanb/simplify-api-generics branch August 9, 2024 09:13
@ltitanb ltitanb mentioned this pull request Aug 9, 2024
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.

4 participants