-
Notifications
You must be signed in to change notification settings - Fork 67
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
chore: upgrade ipfs to 0.55.x #661
base: main
Are you sure you want to change the base?
Conversation
Do I have to create a fleek account to see why CI fails? |
@achingbrain I'm afraid so. |
Ah, right: return bufferedContents.toString() probably needs to change to: return new TextDecoder().decode(bufferedContents) |
Updates to the latest ipfs release. Some extra build config was needed because it uses optional chaining. This is supported out of the box by the version of acorn used by webpack 5 but vue is currently using webpack 4 so some extra config was necessary. Also uprades `to-buffer` - this now returns `Uint8Array`s instead of the node `Buffer` built in, which means you can't call `.toString(encoding)` on it any more, instead you need to use the global js `TextDecoder` class.
074fc9e
to
a23125f
Compare
@terichadbourne ready for your review. Thank you @achingbrain for working on this! 🙏 |
Thank you both for all of your help with this. I just went through each lesson and compared the output of the solution code to the lesson contents about what the method is supposed to return. There were a few methods where this had changed, eg as pointed out in #691), so I updated the lesson text to match the output I actually see. @achingbrain Could I ask you to do me the favor of reviewing my 4 recent commits (all small) to ensure you agree with the way I've described what the user should now expect? In the last commit, there were a few cases where I didn't know how to properly describe what a field meant (or the unit it was presented in and would love your help. (I add TODOs in the code.) The last TODO in the Regular Files API tutorial is a spot where we tell them that the result of Thanks in advance for your help on this! |
@achingbrain Just a ping here - could you find time to review my last 4 commits and make sure they're accurate and answer the question in my comment above? Thanks! |
- `mode`: the UnixFS mode as a Number | ||
- `mtime`: an object with numeric `secs` and `nsecs` properties |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field will be present if found in the DAG so it's probably worth keeping it here with a note that it's an optional property.
That is, mode
gets a default value depending on if the entry is a file or directory, but mtime
does not.
It's probably worth revisiting this exercise. In [email protected] the output of
There's now a clear reason to use |
Updates to the latest ipfs release. Some extra build config was
needed because it uses optional chaining.
This is supported out of the box by the version of acorn used by
webpack 5 but vue is currently using webpack 4 so some extra config
was necessary.
Also uprades
to-buffer
- this now returnsUint8Array
s insteadof the node
Buffer
built in, which means you can't call.toString(encoding)
on it any more, instead you need to use theglobal js
TextDecoder
class.