/snapd

client,cmd/snap: clarify name ambiguity in Plug or Slot #547

Merged
merged 2 commits into from Mar 1, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
Reviewed
Projects
None yet
Milestone
No milestone
3 participants
@zyga @niemeyer @stevenwilkin
@zyga
Contributor

zyga commented Feb 29, 2016

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
@zyga
client,cmd/snap: specialize Name to either 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>

Some checks were not successful

1 failing and 2 successful checks
@snappy-m-o
Integration tests — No test results found.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage remained the same at 70.504%
Details
aba8bbf
@niemeyer
Contributor

niemeyer commented Feb 29, 2016

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.

@niemeyer niemeyer added the Reviewed label Feb 29, 2016

@zyga zyga closed this Mar 1, 2016

@zyga
Contributor

zyga commented Mar 1, 2016

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.
@zyga
client,cmd/snap: use Name for Plug and Slot name 
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>

Some checks were not successful

1 failing and 2 successful checks
@snappy-m-o
Integration tests — 49 tests run, 0 skipped, 1 failed.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.2%) to 70.686%
Details
73e197c

@zyga zyga reopened this Mar 1, 2016

@zyga zyga changed the title from client,cmd/snap: specialize Name to either Plug or Slot to client,cmd/snap: clarify name ambiguity in Plug or Slot Mar 1, 2016

cmd/snap/cmd_interfaces.go
@@ -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 {
@stevenwilkin

stevenwilkin Mar 1, 2016

Member

Would connection.Plug.Name or similar make more sense here? I see that AllPlugs() returns []PlugConnections

@zyga

zyga Mar 1, 2016

Contributor

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.

@stevenwilkin
Member

stevenwilkin commented Mar 1, 2016

👍

zyga added a commit that referenced this pull request Mar 1, 2016

@zyga
Merge pull request #547 from zyga/banish-nonspecific-name-v2 
client,cmd/snap: clarify name ambiguity in Plug or Slot
443e328

@zyga zyga merged commit 443e328 into snapcore:master Mar 1, 2016
2 of 3 checks passed

2 of 3 checks passed

Integration tests Started
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 70.686%
Details

@zyga zyga deleted the zyga:banish-nonspecific-name-v2 branch Mar 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment