-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Added Default logic to refs #2678
Conversation
@@ -50,7 +50,7 @@ NOTE: List all references recursively by using the flag '-r'. | |||
cmds.StringArg("ipfs-path", true, true, "Path to the object(s) to list refs from.").EnableStdin(), | |||
}, | |||
Options: []cmds.Option{ | |||
cmds.StringOption("format", "Emit edges with given format. Available tokens: <src> <dst> <linkname>."), | |||
cmds.StringOption("format", "Emit edges with given format. Available tokens: <src> <dst> <linkname>.").Default("refs"), |
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.
This is wrong.
It should be <dst>
.
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.
Fixed.
3946a96
to
2fb0e23
Compare
LGTM |
@@ -313,6 +313,8 @@ func (rw *RefWriter) WriteEdge(from, to key.Key, linkname string) error { | |||
|
|||
var s string | |||
switch { | |||
case rw.PrintFmt == "<dst>": |
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.
I don't think this case is actually needed, the next case will do what youre doing here already
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 might be a simple (possibly premature) optimization.
99% of uses of this command will use this format string.
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.
That was my thinking: is this a premature opt?
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, i'd call this a premature op. The added time of going through the other case will be dwarfed by the cost of a b58 marshal and api latency
Not sure about the switch statement. There may be a more elegant solution. Part of #2484 License: MIT Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
2fb0e23
to
90421f5
Compare
Fixed. |
Not sure about the switch statement. There may be a more elegant solution.
Part of #2484
License: MIT
Signed-off-by: Richard Littauer richard.littauer@gmail.com