many: replace interfaces.Repository.AddSnap with AddAppSet #13772
Conversation
github-actions
bot
added
the
Needs Documentation -auto-
andrewphelpsj
force-pushed
the
add-app-set
branch
2 times, most recently
from
7098b7c
to
e21b479
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
4 times, most recently
from
bd45c89
to
97b05af
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
4 times, most recently
from
8bf7bd4
to
bdaf052
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
3 times, most recently
from
d09a914
to
7a7134d
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
2 times, most recently
from
dfaa7b0
to
f33e330
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
2 times, most recently
from
4a33c39
to
47a6b55
Compare
snap/component.go
Outdated
|
|
||
| // NewComponentInfo creates a new ComponentInfo. | ||
| func NewComponentInfo(cref naming.ComponentRef, ctype ComponentType, version, summary, description string) *ComponentInfo { |
There was a problem hiding this comment.
Is there any specific reason to remove this function? It is not especially useful right now, but in the past we had problems with not having this sort of function to create structs and we introduced changes like a new field whose value depended partially on other fields, etc. If the struct was created directly in many tests then the changes were very time consuming and annoying.
There was a problem hiding this comment.
This is used much later in “snap run” to set some environs variables
interfaces/repo.go
Outdated
| @@ -1000,10 +953,12 @@ func (r *Repository) AddSnap(snapInfo *snap.Info) error { | |||
|
|
|||
| snapName := snapInfo.InstanceName() | |||
|
|
|||
| if r.plugs[snapName] != nil || r.slots[snapName] != nil { | |||
| if r.appSets[snapName] != nil || r.plugs[snapName] != nil || r.slots[snapName] != nil { | |||
There was a problem hiding this comment.
Should be enough with checking r.appSets[snapName] I guess? Unless AddAppSet is not always called for all snaps.
There was a problem hiding this comment.
You are correct, this invariant should always hold. Would panicking be preferable in this case?
interfaces/repo.go
Outdated
| @@ -973,23 +923,26 @@ func (r *Repository) SnapSpecification(securitySystem SecuritySystem, appSet *Sn | |||
| return spec, nil | |||
| } | |||
|
|
|||
| // AddSnap adds plugs and slots declared by the given snap to the repository. | |||
| // AddAppSet adds plugs and slots declared by the given snap to the repository. | |||
There was a problem hiding this comment.
this doc comment probably need a bit more updates, as the AppSet is not strictly just about the snap anymore
andrewphelpsj
force-pushed
the
add-app-set
branch
from
832f38c
to
eb574d7
Compare
andrewphelpsj
force-pushed
the
add-app-set
branch
from
e0bbf2a
to
31d111d
Compare
This change will enable us to inject an app set into the interfaces.Connected(Plug|Slot) types, which will allow interfaces to correctly generate label expressions for connected plugs/slots.