-
Notifications
You must be signed in to change notification settings - Fork 67
fix: updating to new CID API in latest js-ipfs, fixes #79 #88
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
Conversation
@mikeal I definitely want to review this, in particular to ensure the new instructions are clear and I can pass the lessons using the new API, but I wonder if we should also ask someone more familiar with IPFS and the changed API to give it a quick once-over as well? |
@mikeal I'm going to drop here my failed attempts to pass some of these lessons. It's most likely that I'm missing other concepts, as opposed to your code updates being wrong, but if we can talk about my mistakes it will help me rewrite instructions to be more clear or offer some hints. BASICS Basics Lesson 2 - http://localhost:8080/#/basics/02 Failled attempt:
Basics Lesson 3 The second example block here still shows a Failed attempt 1:
Failed attempt 2:
Failed attempt 3:
Also tried this with a slash before |
@mikeal Comments and my mistakes continued: BLOG ** Blog Lesson 1** Note the misspelling in: The exercise instructions still say "Remember to use toBaseEncodedString() to make the CID links." Nothing happens when I click the "Submit" button, but for reference, this is what I tried as my code. I tried it because of my experience with the past version of the lesson, not because the example above suggested I should do it:
Are big silly strings and actual data interchangeable in how this all works? Could a and b be identical? Which of c or d would be right? I think part of why I'm finding this lesson confusing now because the examples are using crazy strings instead of the simplistic data I saw in most of the Basics lesson. When I try to solve an exercise, I hope there's a code example above that's very similar to what I need. I don't know the correct solution to this exercise in its current form, but if I don't need to type "newCid" in my solution, then I find the example above confusing. I want to try to apply whatever I just saw. In retrospect this was might be what had me confused in Basics Lesson 2. Blog Lesson 2 I passed this one! I'm noticing that
Blog Lesson 3 While looking at this lesson I realized that we've never introduced the term "node," which is used here and was first used in Basics Lesson 1. Is there an easy way to define it, or will it be defined explicitly in the new decentralized concepts workshop, and we could link to it from the other workshops? This is not a change you introduced, just something I'm noticing now:
I think we should include an example so people are sure they understand what a named link is. Would this be an accurate re-phrasing?
Or is a named link something that would have been specifically {'/': whatever} in the old API, which is now a format the user has never been exposed to. Failed code attempt:
|
@mikeal Continued... BLOG Blog Lesson 4 failed attempt 1:
failed attempt 2:
failed attempt 3
submit button appears to user not to work but the following error is logged in the console:
failed attempt 4:
Same result as above if I do Blog Lesson 5 failed attempt 1:
no error message appears to user when button is clicked but the following is in the console:
|
Yup, updated PR title to be clear. |
Oh hey, this works a lot better after I install the new dependencies 🤦♀️ The last lesson is still way harder than the others and is very prone to stalling out the whole site if you get the code wrong, but otherwise I was able to pass the lessons with the new API. Suggested edits for clarity: Basics Lesson 2:
Blog
Lesson 2:
Lesson 3:
Lesson 5:
to
Lesson 7:
@mikeal Anything in these proposals you disagree with? Would you like to make these changes yourself or would you like me to take a stab at it? |
- package-lock.json updated by `npm install`
@terichadbourne go ahead and make changes directly in to the branch and I'll review them before we merge this. |
- Remove a few extraneous references to the old API - Update examples to use links stored as variables, since the 'new CID(string)' format hasn't been introduced and isn't needed in the exercises - Corrected a few grammatical errors - Attempted to clarify instructions across lessons - Moved some text between exercise box and general lesson text for consistency
@mikeal I've made my edits as proposed to remove a few remaining references to the old API and improve clarity of instructions throughout. I'd appreciate it if you could give this a careful once-over before merging in case I actually introduced any inaccuracies while trying to make it more comprehensible. I'll update the issue for blog lesson 7, which is still really hard to solve and needs a little more explanation, although I did drop some clues in here. I'll also add an issue to add another lesson to the basics workshop that shows the base encoded strings and Your API changes seem to be working, so I'm comfortable with you merging if you're good with my edits. |
This is SO much cleaner!