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

introduce IpfsNode Plugin #6719

Merged
merged 1 commit into from
Nov 22, 2019
Merged

introduce IpfsNode Plugin #6719

merged 1 commit into from
Nov 22, 2019

Conversation

MichaelMure
Copy link
Contributor

This PR introduce a new kind of plugin able to directly use IpfsNode. As IpfsNode is considered internal, no guarantee is made about API stability. This is useful when a plugin needs a direct access to the internal and using CoreApi is not enough.

As introducing PluginNode make the naming a little akward, this PR also introduce a copy of PluginDaemon named PluginApi and deprecate PluginDaemon. As it is a separate commit, we can remove that easily if you don't like it.

@MichaelMure MichaelMure changed the title Plugin node introduce IpfsNode Plugin Oct 21, 2019
@MichaelMure
Copy link
Contributor Author

MichaelMure commented Oct 21, 2019

golint fail because PluginDaemon is now deprecated. Other CI failures seem unrelated.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

(finally reviewing this)

Code LGTM I'd like to revert the rename and use:

  • PluginDaemon - uses the API.
  • PluginDaemonInternal - uses the raw node and makes it clear that this is an internal plugin.

I called it "PluginDaemon" instead of "PluginAPI" because it follows the lifecycle of the daemon itself.

@MichaelMure
Copy link
Contributor Author

Updated as you wanted.

@Stebalien
Copy link
Member

Thanks!

@Stebalien Stebalien merged commit 14605f9 into ipfs:master Nov 22, 2019
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.

2 participants