ferristseng / rust-ipfs-api

IPFS HTTP client in Rust
Apache License 2.0
247 stars 68 forks source link

ls_with_options #61

Closed jcaesar closed 3 years ago

jcaesar commented 3 years ago

This adds options to ipfs ls in the style of #54.

In theory, this should be a no-brainer. In practice, there's a small detail: the path parameter to ls is not actually optional, I don't know why it's an Option on IpfsClient::ls, and I'd prefer not to replicate that mistake. I've made it so the API doesn't break. If you want me to break IpfsClient::ls for consistency and correctnesses sake, please tell me.

(Btw, these are rather necessary to list large folders: ipfs ls --stream --resolve-type=false --size=false /ipfs/bafybeicupvauz2urhyg7brbqy37z6ytorxmsog7uns6scntmd3cpvxfnyu/distfiles should complete in about 20 minutes. Remove --stream and you just get no output for a very long time (but it does compelete). Remove the other two, too, and my node crashes. Though admittedly, folders with 10000s of items are rare.)

ferristseng commented 3 years ago

In practice, there's a small detail: the path parameter to ls is not actually optional, I don't know why it's an Option on IpfsClient::ls, and I'd prefer not to replicate that mistake. I've made it so the API doesn't break. If you want me to break IpfsClient::ls for consistency and correctnesses sake, please tell me.

Thanks for catching that mistake. I think just breaking the API is fine here for correctness sake.

Otherwise, looks great to me! I'll merge it when you make that change.

jcaesar commented 3 years ago

I think just breaking the API is fine here for correctness sake.

Done. (I wonder if it's ok to have api-breaking changes between two RCs, but probably wouldn't mind even if it wasn't ok.)

ferristseng commented 3 years ago

I think just breaking the API is fine here for correctness sake.

Done.

Excellent, thank you!

(I wonder if it's ok to have api-breaking changes between two RCs, but probably wouldn't mind even if it wasn't ok.)

The version-ing is kind of a mess right now, and that's my fault. I think I'm just going to go back to minor versions and not mess around with RCs. It sounds like public API changes are fine in pre 1.0 releases regardless: https://semver.org/#spec-item-4

jcaesar commented 3 years ago

TIL. Thank you.