Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

docs: examples/browser-create-react-app #3694

Merged
merged 8 commits into from
Jul 5, 2021
Merged

docs: examples/browser-create-react-app #3694

merged 8 commits into from
Jul 5, 2021

Conversation

kvutien
Copy link
Contributor

@kvutien kvutien commented May 18, 2021

Hello

Thanks for this tutorial. I learned a lot from it.

  • I added explanatory comments in the code to help a future newbie like me spin up.
  • I changed the README.md that still had legacy parts from create-react-app that are not applicable here.

Best.
Vu Tien Khang
Luxembourg

@welcome

This comment has been minimized.

@lidel
Copy link
Member

lidel commented Jun 25, 2021

@kvutien thank you! are you able to rebase this PR and fix CI errors?

@lidel lidel added the need/author-input Needs input from the original author label Jun 25, 2021
@kvutien
Copy link
Contributor Author

kvutien commented Jun 25, 2021

Thank you for the question but I'm afraid it's above my level of understanding.

I'm a beginner and I just changed the README. It contained plenty of stuff from the original create-react-app README that are not relevant to this short demo.

From my working notes of May, the rest of the code worked well enough straight as cloned, so I did not need to do changes. Congrats for using React hooks.

  • What means "rebase a Pull Request"?
  • I haven't use the CI functions

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

No worries @kvutien, thanks for proof-reading and improving this example as a beginner, it is very valuable feedback 👍

I applied small changes to some sentences and added some links, but overall looks good to me.

FYI every example in this repo has tests that run on CI and here you can see the test failed because a label changed.
I fixed it and CI is now green.

examples/browser-create-react-app/README.md Outdated Show resolved Hide resolved
examples/browser-create-react-app/README.md Outdated Show resolved Hide resolved
examples/browser-create-react-app/README.md Outdated Show resolved Hide resolved
examples/browser-create-react-app/README.md Outdated Show resolved Hide resolved
examples/browser-create-react-app/README.md Outdated Show resolved Hide resolved
@lidel lidel changed the title examples/browser-create-react-app docs: examples/browser-create-react-app Jul 2, 2021
@lidel lidel removed their assignment Jul 2, 2021
@lidel lidel removed the need/author-input Needs input from the original author label Jul 2, 2021
@kvutien
Copy link
Contributor Author

kvutien commented Jul 3, 2021

Great. Thanks @lidel . Happy to help in anyway.

On the other hand, I think that you are starting an interesting debate: should a tutorial facilitate the life of the developer team (project IPFS) or the life of the beginner? or of the evangelizer, almost beginner? 😉 for example...

  • giving the date of the tutorial and the version of the tools helps the beginner make sure he/she is using the right dev environment instead of pulling their hair. See the number of Stack Exchange questions that are solved by answers like 'use node v12' or 'use ipfs 0.40' etc. Breaking changes do happen.
  • Self hosting is nice, but you can't imagine how perplex I was when I ran on some blog tutorials that refer to node addresses that don't exist anymore, hard coded either in the code, in the config bootstrap or config swarm. Self-hosting yet another thing to add to the cognitive load of a beginner. That's why services like Infura and Pinata exist and are good.
  • considering that the IPFS/OrbitDB team is spread out on so many topics and languages, wouldn't it be useful to have a synergy by linking to (good) external web sites that concur to the same objective than yours? There are as many chances that their content is out of sync than the content of the ipfs repo itself 🙂

My 2 cents...

@lidel
Copy link
Member

lidel commented Jul 5, 2021

Thanks! Quick closing thoughts:

  • Examples in this repo are provided in a way that minimizes maintenance cost. Their main purpose is to demonstrate use case and to run regression tests, to ensure they always work with the latest js-ipfs. Educational aspect, while important, comes second. While the things you mentioned are nice to have, they introduce additional manual labor, and the team has limited resources.
  • Having too many "sources of truth" is also confusing to usersm makes it harder for newcomers to discover valuable ones. We suggest improving https://docs.ipfs.io – you can submit changes and new articles via PR against https://github.com/ipfs/ipfs-docs 👍

@kvutien
Copy link
Contributor Author

kvutien commented Jul 5, 2021

100% agree.

Checking my working notes, I see that

  1. you are right, the version discrepancies I met in the code were mainly in the orbitdb repo, not ipfs
  2. or in "sources of truth" from blogs, outside ifps
  3. but I had recourse on them because when I started I could not manage to understand the contents of docs.ipfs.io. Today, after one month of exploration and after ProtoSchool, I understand better. I'll try and see how the pages that I found difficult in docs.ipfs.io can be modified.

FYI every example in this repo has tests that run on CI and here you can see the test failed because a label changed.
I fixed it and CI is now green.

I'd love to learn more about your CI. Is it in .travis.yml? Anyhow, it seems that the Travis and lerna build failed. Only 6 of 22 packages passed.

outdated (window.ipfs is no longer used) + we dont want to maintain it
when the app gets refactored in the future
@lidel lidel merged commit dc041aa into ipfs:master Jul 5, 2021
oliveriosousa pushed a commit to oliveriosousa/js-ipfs that referenced this pull request Jul 5, 2021
Co-authored-by: kvutien <kvutien@VTKT5.fritz.box>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
oliveriosousa pushed a commit to oliveriosousa/js-ipfs that referenced this pull request Jul 26, 2021
Co-authored-by: kvutien <kvutien@VTKT5.fritz.box>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants