-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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. |
@@ -15,6 +15,8 @@ updateNotifier({ | |||
yargs | |||
.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 <jacobheun@gmail.com>
Aliases add, cat and get to be in the top-level namespace of the cli.
Reference: #455 (comment)
Left to do: