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

interledger-ccp API review for v1.0-rc #571

Closed
8 tasks done
gakonst opened this issue Dec 18, 2019 · 2 comments
Closed
8 tasks done

interledger-ccp API review for v1.0-rc #571

gakonst opened this issue Dec 18, 2019 · 2 comments
Assignees
Labels
crate/interledger-ccp docs v1.0 Issues which need to be handled for a v1.0 release

Comments

@gakonst
Copy link
Member

gakonst commented Dec 18, 2019

interledger-ccp public items marked with x are documented. Non-exported items are not taken into account. Follows format from #561.

Part of #557.

  • CcpRoutingAccount (trait)
  • RouteManagerStore (trait):
    • Should this be renamed to CcpRoutingStore? That would make it consistent with how the other Account/Store pairs are named.
  • RoutingRelation
  • Mode
  • RouteControlRequest
    • Should to_prepare be implemented as Prepare::from(src: RouteControlRequest)?
  • RouteProp
    • Should we convert it to pub(crate)?
  • CcpRouteManager
  • CcpRouteManagerBuilder: Does this need to be documented? Its fields are docuemnted, and given that it's a builder its functionality should be intuitive.
@gakonst gakonst added this to To do in Towards a v1.0 Dec 18, 2019
@gakonst gakonst mentioned this issue Jan 20, 2020
11 tasks
@gakonst gakonst moved this from To do to In progress in Towards a v1.0 Jan 20, 2020
@gakonst gakonst self-assigned this Jan 20, 2020
@gakonst gakonst added v1.0 Issues which need to be handled for a v1.0 release docs labels Jan 20, 2020
@gakonst gakonst moved this from In progress to Waiting Review / Response in Towards a v1.0 Jan 20, 2020
@bstrie
Copy link
Contributor

bstrie commented Feb 4, 2020

  • I'd say yes to the rename of RouteManagerStore.
  • No opinion on Mode, do whatever seems most conservative.
  • If RouteProp can be made pub(crate), then absolutely. Reducing the public surface area is good.
  • CcpRouteManagerBuilder: Even just a line saying something like "See documentation on fields" would be nice to show that documentation wasn't just forgotten about.

@gakonst
Copy link
Member Author

gakonst commented Feb 4, 2020

  • Renamed to CcpRoutingStore
  • Mode has been left as is, for potential future implementation of the Idle feature, since it's implemented in the js connectors
  • RouteProp is now pub(crate)
  • Added a small comment on CcpRouteManagerBuilder

@gakonst gakonst closed this as completed Feb 4, 2020
Towards a v1.0 automation moved this from Waiting Review / Response to Done Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/interledger-ccp docs v1.0 Issues which need to be handled for a v1.0 release
Projects
No open projects
Development

No branches or pull requests

2 participants