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

docs: clarify that mtime and mode are optional #3302

Merged
merged 2 commits into from
Oct 27, 2020

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 24, 2020

This should minimize the number of people who pass mtime and mode to ipfs.add when they don't really need them.

Ref. https://discuss.ipfs.io/t/why-has-same-sub-cids-but-not-same-root-cid/8810/6?u=lidel

This should minimize the number of people who pass those
to ipfs.add when they don't really need them
@@ -180,7 +180,7 @@ An optional object which may have the following keys:
| -------- | -------- |
| `Promise<UnixFSEntry>` | A object describing the added data |

Each yielded object is of the form:
Each yielded object is of the form (`mtime` and `mode` are optional, so may be missing):
Copy link
Member

Choose a reason for hiding this comment

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

mode has a default value so shouldn't be missing.

https://github.com/ipfs/specs/blob/master/UNIXFS.md#metadata

Copy link
Member Author

Choose a reason for hiding this comment

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

@achingbrain Ack, I was unaware of implicit mode default.
This complicates things a bit.. would this work?

Suggested change
Each yielded object is of the form (`mtime` and `mode` are optional, so may be missing):
Each yielded object is of the form (with optional `mtime` and custom/implicit `mode` [metadata](https://github.com/ipfs/specs/blob/master/UNIXFS.md#metadata)):

If not, we could just leave this line as it was before. Let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps reverting this and just adding a comment in the JS object below would be helpful? Additionally, keeping the optional ? indicator on the key is a nice way to represent the potential missing response value.

{
  path: '/tmp/myfile.txt',
  cid: CID('QmHash'),
  mode: Number, // implicit if not provided
  mtime?: { secs: Number, nsecs: Number }, // may be missing
  size: 123
}

Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

See comment inline

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants