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

Refactor Simple Chat App tutorial #964

Closed
wants to merge 1 commit into from
Closed

Conversation

gmelodie
Copy link

@gmelodie gmelodie commented Dec 6, 2021

This relates to #798. I had a few thoughts but figured it'd be easier to create a PR so you can see what I mean more concretely rather than discussing it on the issue. Hope you enjoy it. Looking forward to hearing back :)

Below you'll find a summary of changes I made.

Getting the code

  1. Move live demo link from "Getting the code" to very top section.

Peer discovery and connectivity:

  1. Add links to circuit relay docs and WebRTC-star
  2. Color indicators are now an unordered list

Docker

  1. Move "After this section ..." to the section above
  2. Include a link to certbot
  3. Replace DOMAIN.COM with YOURDOMAIN.COM (more obvious, and used later on)

WebRTC-Star

  1. Update links to webrtc-star docs (currently archived and deprecated, might be worth to revisit)

p2p-circuit

  1. Add a better intro to shortly explain why we would need circuits and how it works in a high level
  2. Reduce text from usage explanation to make it cleaner
  3. Add tip about the ipfs.thedisco.zone domain
  4. Edit setup description to shortly explain websockets beforehand
  5. Make advertising script section more interactive (more steps)

Communication

  1. discochat-global typo, should be example_topic as per code

Possible browser pitfalls

  1. Make text under "Staying connected to peers" more direct

@welcome
Copy link

welcome bot commented Dec 6, 2021

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions and stakeholders linked/mentioned.
  • Your contribution itself is clear (grammar and spelling checked, any code blocks checked and commented) and in its best form. Follow the docs contribution guidelines if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on any missing things and potentially assigning a reviewer for high priority items.
  • The PR gets reviews, discussed and approvals as needed.
  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

@lidel
Copy link
Member

lidel commented Dec 16, 2021

@gmelodie thanks! will you have time to also test with go-ipfs 0.11? If you dont have time to dig too deep into 0.11 changes, for now adding a single sentence informing users that EnableRelayHop works only in go-ipfs <0.11 will be enough 🙏

TLDR: enabling relay v1 with EnableRelayHop no longer works in go-ipfs 0.11.0 (see breaking changes). We need to update this tutorial to point folks at relay v1 from js-ipfs or a standalone libp2p-relay-daemo running as RelayV1.

@BigLep BigLep added the need/author-input Needs input from the original author label Apr 22, 2022
@BigLep
Copy link
Contributor

BigLep commented Apr 22, 2022

@gmelodie : are you able to incorporate the feedback? Thanks.

@gmelodie
Copy link
Author

Hey! I'm a bit busy with work for now, if someone's able to make those changes that'd be awesome!

@johnnymatthews
Copy link
Contributor

If this is still being worked on, I'm gonna change this to a draft.

@johnnymatthews johnnymatthews marked this pull request as draft April 28, 2022 02:01
@TMoMoreau
Copy link
Contributor

With a new version of the chat app, an entirely new guide/tutorial is needed, see issue #1459. Closing this PR.

@TMoMoreau TMoMoreau closed this Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/stale need/author-input Needs input from the original author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants