-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(cli): alias add, cat and get to top-level cli #493
Conversation
|
We also need this for the http api. Also please add some sanity tests to check these actually exist and work. |
|
@dignifiedquire oh, wasn't aware of that. All right, adding a TODO in PR. |
src/cli/bin.js
Outdated
| .commandDir('commands') | ||
| .demand(1) | ||
| // Temporary alias of {add,cat,get} | ||
| .commandDir('commands/files') |
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.
It is more the other way around. {add, cat, get} are temporary alias of { fileas add, files cat, files get}
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.
@diasdavid Hm, probably have to change the wording. My understanding is that files add, files cat and files get is here to stay. add, cat and get is what is the aliases and the code here is aliasing add/cat/get to files add/files cat/files get but the comment doesn't seem to be informative enough.
Have any ideas on how to improve it?
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.
What you just said is correct :)
You can replace the comment to
// NOTE: This creates an alias of `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. This will stay until https://github.com/ipfs/specs/issues/98 is resolved.
Which API? The core API should follow the interface-ipfs-core spec, which only defines files |
|
I think the http-api. |
|
@diasdavid can you please take a look at my comment: #493 (comment) And answer if this has to be for the http-api as well |
|
@victorbjelkholm answered :) |
|
@victorbjelkholm #488 is blocked on this, anything blocking you? |
|
@diasdavid no, I'll get this ready asap |
2d8547c to
d0141b3
Compare
|
@diasdavid done with adding tests etc, ready to merge as soon as travis finishes. |
|
@victorbjelkholm I also need those on the http-api |
|
@victorbjelkholm it is missing a |
|
Hey! Seems that this one got lost in time, can we get it shipped soon? |
|
@diasdavid yes, absolutely, just missing one test and then it's ready. I dropped the ball on this and I'll jump right on it again! |
29dc5c4 to
f675fc6
Compare
f675fc6 to
6ad325b
Compare
|
Added test, so this is ready to merge now @diasdavid @dignifiedquire |
|
Sweeeet 🍰 ! :D |
License: MIT Signed-off-by: Jacob Heun <[email protected]>
Aliases add, cat and get to be in the top-level namespace of the cli.
Reference: #455 (comment)
Left to do: