-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add can store rm
& can upload rm
commands
#101
Conversation
adds impl, tests, and docs for the `can store rm` and `can upload rm` commands License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>
License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>
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.
LGTM - a few suggestions but they are non blocking 🙂
if (cid.version === 1 && cid.code === CAR.codec.code) { | ||
return /** @type {CARLink} */ (cid) | ||
} | ||
} |
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.
Alternatively you could make this a type assertion function:
/**
* Return true if CID is a CAR CID and false if not.
* @param {import('multiformats').UnknownLink} cid
* @returns {cid is import('multiformats').Link<Uint8Array, typeof CAR.codec.code>}
*/
export function isCarLink (cid) {
return cid.version === 1 && cid.code === CAR.codec.code
}
...and then typescript knows it's a CAR CID later in the code and you don't have to re-assign e.g.:
lib.js
Outdated
error(`Error: ${cidStr} is not a CAR CID`) | ||
} | ||
return car | ||
} catch (err) { |
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.
} catch (err) { | |
} catch { |
lib.js
Outdated
const cid = CID.parse(cidStr.trim()) | ||
const car = asCarLink(cid) | ||
if (car === undefined) { | ||
error(`Error: ${cidStr} is not a CAR CID`) |
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.
Kinda weird to destructure error
from console...
I'd probably just have it throw when not a CAR CID and then in the calling code:
let cid
try {
cid = parseCarLink(cidStr)
} catch {
console.error(`Error: invalid CAR CID: ${cidStr}`)
process.exit(1)
}
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.
yeah it used to be parseCarLinkOrExit
but that was weird too. Have simplified it again.
export function parseCarLink (cidStr) {
try {
return asCarLink(CID.parse(cidStr.trim()))
} catch {}
}
calling code
const shard = parseCarLink(cidStr)
if (!shard) {
console.error(`Error: ${cidStr} is not a CAR CID`)
process.exit(1)
}
License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>
🤖 I have created a release *beep* *boop* --- ## [4.3.0](v4.2.1...v4.3.0) (2023-10-18) ### Features * add --verbose --json option to upload command and print piece CID ([#97](#97)) ([775d1db](775d1db)) * add `can store rm` & `can upload rm` commands ([#101](#101)) ([a7bda04](a7bda04)) * introduce unified service config ([#99](#99)) ([f3b6220](f3b6220)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
adds impl, tests, and docs for the
can store rm
andcan upload rm
commands.License: MIT