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

fix: Change Readme contents to reflect implementation #287

Merged
merged 2 commits into from Oct 15, 2018
Merged

Conversation

ltfschoen
Copy link
Contributor

I had a closer look at the code to see what Node Interface methods we are wrapping in @polkadot/api-observable, and which Node Interface methods we are wrapping in @polkadot/rpc-core, and I think these changes are appropriate.

The link to the package @polkadot/rpc-rx that we are using appears to have been accidently removed previously, so I've added it back again.

Note importantly that @polkadot/api contains @polkadot/api/rx, whereas @polkadot/rpc-rx is a different separate package

@jacogr
Copy link
Member

jacogr commented Oct 14, 2018

Yes, api-observable needs to -

  • start using api/rx
  • remove all the single-use methods, only contain wrappers (i.e. all the stuff where combine is used)

rpc-rx, like rpc-core needs to be on this list, but it one of those things we probably don't want use of directly unless people know what they are doing - they only take RPC calls and either make them promises or observables.

EDIT: "accidentally removed" - that is my overzealous editing when I saw the name "api-rx" (all before morning coffee) ;)

@jacogr jacogr merged commit b2c512f into master Oct 15, 2018
@jacogr jacogr deleted the luke-fix-readme branch October 15, 2018 21:15
@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants