-
Notifications
You must be signed in to change notification settings - Fork 573
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
client,cmd/snap: clarify name ambiguity in Plug or Slot #547
Conversation
This patch changes the public snappy API to refer to Plug.Name as .Plug and Slot.Name as .Slot. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Hmm.. that doesn't look like an improvement to me. The reasoning for using it at the API edge is to reinforce the terminology and make the data more obvious. Inside our own API, having Plug.Plug will likely not help much, comparing it to Plug.Name. |
I'm closing this temporarily. After discussing this online we agreed to use mixed json/go field naming. For Go we'll have Plug.Name and Slot.Name but in JSON we'll have {"plug": "..."} and {"slot": "..."} to differentiate the two fields. |
This patch differentiates the Go and JSON interfaces to Plug and Slot names. This comes after a discussion that wanted to eliminate the ugly Plug.Plug "bork bork bork"-looking code without sacrificing the unambiguity in the typeless JSON representation. With this patch, the on-the-wire protocol remains the same, unambiguous {"plug": "..."} and {"slot": "..."}. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -68,18 +68,18 @@ func (x *cmdInterfaces) Execute(args []string) error { | |||
if x.Positionals.Query.Snap != "" && x.Positionals.Query.Snap != plug.Snap { | |||
continue | |||
} | |||
if x.Positionals.Query.Name != "" && x.Positionals.Query.Name != plug.Name { | |||
if x.Positionals.Query.Name != "" && x.Positionals.Query.Name != plug.Plug.Name { |
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.
Would connection.Plug.Name
or similar make more sense here? I see that AllPlugs()
returns []PlugConnections
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 fixed with the upcoming /2.0/interfaces API change. It actually returns InterfaceConnections
then but you'll have to wait a sec to see it.
👍 |
client,cmd/snap: clarify name ambiguity in Plug or Slot
This patch changes the public snappy API to refer to Plug.Name as .Plug
and Slot.Name as .Slot.
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com