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

Update companion-node-types.md #581

Merged
merged 22 commits into from
Jan 7, 2021

Conversation

jessicaschilling
Copy link
Contributor

@jessicaschilling jessicaschilling commented Jan 5, 2021

Ready to merge once approved.

This PR updates https://docs.ipfs.io/how-to/companion-node-types/ for two reasons:

  1. Adds info on native browser support in Brave
  2. Rewrites section on embedded + chrome.sockets as deprecated

Copy link
Contributor

@johnnymatthews johnnymatthews left a comment

Choose a reason for hiding this comment

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

One tiny change, otherwise everything looks good. Merge when ready.

jessicaschilling and others added 2 commits January 5, 2021 14:33
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
@lidel lidel requested a review from johnnymatthews January 6, 2021 15:46
@lidel
Copy link
Member

lidel commented Jan 6, 2021

@johnnymatthews @jessicaschilling mind giving it another 👀 ?

  • I've expanded "Native" section a bit with more info why "Provided by Brave" is cool
    • For now Brave is the only one under "Native", but we expect the list to grow, TOC now reflects that
  • Moved "Embedded" to the end, as its the least useful one + added warning + removed mentions of deprecated window.ipfs

@jessicaschilling
Copy link
Contributor Author

@lidel Did a close edit of your revisions in d965c04 - thanks for the additional work!

@johnnymatthews do you mind another look? 🙏

Clarify Brave version with native IPFS
@lidel
Copy link
Member

lidel commented Jan 7, 2021

Added Brave version info to avoid user confusion when they use older one without Native IPFS (current Stable is v1.18):

2021-01-07--16-52-33

Ok to merge on my end: ipfs/ipfs-companion#956 already shipped in ipfs-companion v2.16.0.990

Copy link
Contributor

@johnnymatthews johnnymatthews left a comment

Choose a reason for hiding this comment

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

Mostly minor tweaks. We should probably have a flowchart or table at the top of this page to quickly show readers which node type they should use. It essentially boils down to:

Do you have an IPFS node installed?
|
| --- yes --> Use EXTERNAL mode.
|
| --- no --> Do you have Brave installed?
	     |
	     | --- yes --> Use NATIVE mode within Brave.
	     |
	     | --- no --> Install a local IPFS node, or install Brave.

docs/how-to/companion-node-types.md Show resolved Hide resolved
docs/how-to/companion-node-types.md Outdated Show resolved Hide resolved
docs/how-to/companion-node-types.md Outdated Show resolved Hide resolved

- Use the [IPFS Desktop](https://github.com/ipfs-shipyard/ipfs-desktop) GUI app (for Windows/Linux/Mac), which installs and manages a local IPFS node for you
- Use the [IPFS Desktop](https://github.com/ipfs-shipyard/ipfs-desktop) app (for Windows/Linux/Mac), which installs and manages a local IPFS node for you as well as offering an easy, convenient user interface for managing files, peers, and more
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Use the [IPFS Desktop](https://github.com/ipfs-shipyard/ipfs-desktop) app (for Windows/Linux/Mac), which installs and manages a local IPFS node for you as well as offering an easy, convenient user interface for managing files, peers, and more

Copy link
Contributor

Choose a reason for hiding this comment

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

An above suggestion makes this line redundant.

Comment on lines 17 to 19
- If you prefer a more hands-on approach:
- Install IPFS by following the [command line quick-start guide](command-line-quick-start.md)
- Or run it in [Docker](https://github.com/ipfs/go-ipfs#running-ipfs-inside-docker)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- If you prefer a more hands-on approach:
- Install IPFS by following the [command line quick-start guide](command-line-quick-start.md)
- Or run it in [Docker](https://github.com/ipfs/go-ipfs#running-ipfs-inside-docker)

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, an above suggestion makes these lines redundant.


This node type is only for development and experimentation. Most users should use [external](#external) or [native](#native) node types instead.

:::

Power users can provide [custom config](https://github.com/ipfs/js-ipfs#faq) (e.g. to enable experimental pubsub) via the IPFS Companion [Preferences](https://user-images.githubusercontent.com/157609/38084660-0b97c0cc-334e-11e8-9368-823345ced67f.png)

**Note:** At present, embedded js-ipfs running within webextension (browser context) comes with some limitations:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**Note:** At present, embedded js-ipfs running within webextension (browser context) comes with some limitations:
There are some limitation when running js-ipfs within a browser context:


This node type is only for development and experimentation. Most users should use [external](#external) or [native](#native) node types instead.

:::

Power users can provide [custom config](https://github.com/ipfs/js-ipfs#faq) (e.g. to enable experimental pubsub) via the IPFS Companion [Preferences](https://user-images.githubusercontent.com/157609/38084660-0b97c0cc-334e-11e8-9368-823345ced67f.png)

**Note:** At present, embedded js-ipfs running within webextension (browser context) comes with some limitations:

- Can't act as an HTTP gateway (extension uses public one as a fallback)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Can't act as an HTTP gateway (extension uses public one as a fallback)
- Companion cannot act as an HTTP gateway: the extension uses public one as a fallback.


Power users can provide [custom config](https://github.com/ipfs/js-ipfs#faq) (e.g. to enable experimental pubsub) via the IPFS Companion [Preferences](https://user-images.githubusercontent.com/157609/38084660-0b97c0cc-334e-11e8-9368-823345ced67f.png)

**Note:** At present, embedded js-ipfs running within webextension (browser context) comes with some limitations:

- Can't act as an HTTP gateway (extension uses public one as a fallback)
- Known to be CPU-hungry
([#450](https://github.com/ipfs-shipyard/ipfs-companion/issues/450), [ipfs/js-ipfs#1190](https://github.com/ipfs/js-ipfs/issues/1190)) over time, which may drain your battery
- Known to be CPU-hungry ([#450](https://github.com/ipfs-shipyard/ipfs-companion/issues/450), [ipfs/js-ipfs#1190](https://github.com/ipfs/js-ipfs/issues/1190)) over time, which may drain your battery
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Known to be CPU-hungry ([#450](https://github.com/ipfs-shipyard/ipfs-companion/issues/450), [ipfs/js-ipfs#1190](https://github.com/ipfs/js-ipfs/issues/1190)) over time, which may drain your battery
- The extension is CPU-hungry and may drain your battery. GitHub issues [ipfs-companion #450](https://github.com/ipfs-shipyard/ipfs-companion/issues/450) and [js-ipfs #1190](https://github.com/ipfs/js-ipfs/issues/1190) address this problem.

docs/how-to/companion-node-types.md Outdated Show resolved Hide resolved
Comment on lines 100 to 102
A _public_ node is used as an implicit fallback for gateway functionality when an external node is offline or an embedded node is used. It does not expose the API port.

Because it's a fallback, it's not included as an option in Companion's preferences.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A _public_ node is used as an implicit fallback for gateway functionality when an external node is offline or an embedded node is used. It does not expose the API port.
Because it's a fallback, it's not included as an option in Companion's preferences.
A _public_ node is used as a fallback for gateway functionality when an external node is offline or an embedded node is used. It does not expose the API port. This type of node is not included as an option in Companion's preferences.

jessicaschilling and others added 13 commits January 7, 2021 09:26
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
jessicaschilling and others added 4 commits January 7, 2021 09:48
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
Co-authored-by: Johnny <9611008+johnnymatthews@users.noreply.github.com>
@jessicaschilling
Copy link
Contributor Author

@johnnymatthews - incorporated your suggestions (but leaving the auto-generated list of node types at the top of the page so we know it's always up to date). Please merge if you're cool with it. 😊

cc @lidel

@johnnymatthews johnnymatthews changed the base branch from master to pre-production January 7, 2021 19:23
@johnnymatthews johnnymatthews merged commit bf9671c into pre-production Jan 7, 2021
@johnnymatthews johnnymatthews deleted the chore/update-companion-nodetype-brave branch January 7, 2021 19:23
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