interfaces/builtin: distribute code of touching allInterfaces #3291

Merged
merged 3 commits into from May 10, 2017

Conversation

Projects
None yet
4 participants
Contributor

zyga commented May 10, 2017

This patch moves code that registers an interface in allInterfaces, as
well as the code that ensures a given interface is registered into the
modules related to the actual interface. This should lessen the
conflicts around adding a new interface and simplify the task
conceptually.

The only remaining centralized elements are base declaration entries
(per jdstrand's request) and the list of implicit interfaces (which
could be merged into interfaces easily later).

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

interfaces/builtin: distribute code of touching allInterfaces
This patch moves code that registers an interface in allInterfaces, as
well as the code that ensures a given interface is registered into the
modules related to the actual interface. This should lessen the
conflicts around adding a new interface and simplify the task
conceptually.

The only remaining centralized elements are base declaration entries
(per jdstrand's request) and the list of implicit interfaces (which
could be merged into interfaces easily later).

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

Some questions, code itself looks fine!

// Interfaces returns all of the built-in interfaces.
func Interfaces() []interfaces.Interface {
+ if !sorted {
+ sort.Sort(byIfaceName(allInterfaces))
@mvo5

mvo5 May 10, 2017

Collaborator

Curious, why do we care about sorting?

@zyga

zyga May 10, 2017

Contributor

We don't care about sorting at all but I wanted to ensure that it's not changing this property. The old list used to be sorted so this one can be as well.

+
+// registerIface appends the given interface into the list of all known interfaces.
+func registerIface(iface interfaces.Interface) {
+ allInterfaces = append(allInterfaces, iface)
@mvo5

mvo5 May 10, 2017

Collaborator

Should we care about duplication here? I.e. do not allow to register an interface twice?

@zyga

zyga May 10, 2017

Contributor

Yes, I was thinking about it as well. Doing this branch has uncovered a number of smaller things that ought to be improved and I'm already preparing another one (and I have ideas for two more).

Looks good, +1!

zyga added some commits May 10, 2017

Merge branch 'master' of github.com:snapcore/snapd into tweak/less-co…
…nflicts-around-all-go

This also fixes broken master after API changes related to connection
attributes were merged without adding the required tests that ensure
legacy APIs are no longer used.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/builtin: ensure that older definers are gone
This patch extends the legacy API tests to look for the specification
defines that didn't have connection attributes. This should ensure that
we don't randomly merge something that breaks master.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@chipaca chipaca merged commit 90736a5 into snapcore:master May 10, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:tweak/less-conflicts-around-all-go branch May 10, 2017

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