feat: support name in ls #260
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like there's just a timeout on the windows tests? |
|
Thanks for submitting this - could you please add a test or some assertions to an existing test in https://github.com/ipfs/js-kubo-rpc-client/blob/main/test/interface-tests/src/pin/ls.ts to ensure this works as expected and there will be no regressions in future? |
|
Sure can, however I'm having some issues installing dependencies locally. |
|
Actually that seems to be an issue with the IDE I was using. Got it working now. |
|
No, I think the infrastructure is flaky 🙄. It was 504ing for me too when you posted that, someone's rebooted something and it works now. |
|
How do I test only a specific file with aegir btw? % npm t -- -f test/interface-tests/src/pin/ls.ts
> [email protected] test
> aegir test -f test/interface-tests/src/pin/ls.ts
build
> [email protected] build
> aegir build
[15:19:59] tsc [started]
[15:20:01] tsc [completed]
[15:20:01] esbuild [started]
[15:20:01] esbuild [completed]
test node.js
Exception during run: TypeError: Unknown file extension ".ts" for /Users/joel/code/js-kubo-rpc-client/test/interface-tests/src/pin/ls.ts
at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
at defaultLoad (node:internal/modules/esm/load:141:22)
at ModuleLoader.load (node:internal/modules/esm/loader:409:7)
at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:45)
at link (node:internal/modules/esm/module_job:76:21) {
code: 'ERR_UNKNOWN_FILE_EXTENSION'
}
Command failed with exit code 1: mocha test/interface-tests/src/pin/ls.ts --ui bdd --require source-map-support/register --timeout=60000 --bail |
|
You can use --grep to match strings from You can also stick a |
|
The tests look good - thanks for adding them. I think it just needs an assertion that the If I read it correctly you could just add it to the expect(pinset).to.deep.include({
type: 'recursive',
cid: fixtures.files[0].cid,
name: fixtures.files[0].pinName // <- add this line
}) |
|
So interestingly Kubo doesn't return |
achingbrain
reviewed
Feb 14, 2025
| } | ||
|
|
||
| export interface PinLsResult { | ||
| cid: CID | ||
| type: PinType | string | ||
| metadata?: Record<string, any> | ||
| name?: string |
There was a problem hiding this comment.
If Kubo doesn’t return this field, does it need to be in the interface?
There was a problem hiding this comment.
Kubo does return it if you call pin.ls with the name param. Otherwise not. So yes it needs to be there as optional.
There was a problem hiding this comment.
I think the reason it's not returned by default is that one CID can have multiple names it's pinned under.
There was a problem hiding this comment.
Aha, gotcha - so it just needs a test that shows this then all is good.
There was a problem hiding this comment.
Alright, so now we test for both cases:
should list all types of pinstests thatnameisn't returned in the regular caseshould list multiple partially matched named pinsandshould list specific named pintests that name is returned
There was a problem hiding this comment.
That's perfect, thanks for making those changes.
achingbrain
approved these changes
Feb 15, 2025
github-actions bot
pushed a commit
that referenced
this pull request
Feb 15, 2025
## [5.1.0](v5.0.2...v5.1.0) (2025-02-15) ### Features * support name in pin.ls ([#260](#260)) ([df1d483](df1d483)), closes [/docs.ipfs.tech/reference/kubo/rpc/#api-v0](https://github.com/ipfs//docs.ipfs.tech/reference/kubo/rpc//issues/api-v0) ### Dependencies * bump @multiformats/multiaddr-to-uri from 10.1.2 to 11.0.0 ([#249](#249)) ([dafa529](dafa529)) * bump parse-duration from 1.1.2 to 2.1.2 ([#257](#257)) ([3d28859](3d28859)) * **dev:** bump nock from 13.5.6 to 14.0.1 ([#259](#259)) ([06f6f1e](06f6f1e))
|
🎉 This PR is included in version 5.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Thanks @achingbrain ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to the Kubo docs (and practical experience) the pin label (name) is returned top level on the pin item (not in Metadata)
Also doesn't seem like
Metadatais a thing anymore, but left it in case.