Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
interfaces: compose the base declaration from interfaces #3444
Conversation
All checks have passed
| @@ -142,6 +142,9 @@ type MetaData struct { | ||
| ImplicitOnCore bool `json:"implicit-on-core,omitempty"` | ||
| // ImplicitOnClassic controls if a slot is automatically added to classic systems. | ||
| ImplicitOnClassic bool `json:"implicit-on-classic,omitempty"` | ||
| + | ||
| + BaseDeclarationPlugs string |
| + // Trim newlines at the end of the string. All the elements may have | ||
| + // spurious trailing newlines. All elements start with a leading newline. | ||
| + // We don't want any blanks as that would no longer parse. | ||
| + tr := func(s string) string { return strings.TrimRight(s, "\n") } |
morphis
Jun 8, 2017
Contributor
Would like a better name for tr here as it doesn't say much if you look at something like tr(baseDeclarationHeader). Something like trimTrailingNewLine or so would make it obvious fo the reader.
| @@ -760,3 +761,14 @@ plugs: | ||
| err = cand.Check() | ||
| c.Check(err, NotNil) | ||
| } | ||
| + | ||
| +func (s *baseDeclSuite) TestComposeBaseDeclaration(c *C) { | ||
| + decl, err := policy.ComposeBaseDeclaration(nil) |
zyga
added some commits
Jun 8, 2017
Some checks were not successful
codecov-io
commented
Jun 8, 2017
•
Edited 1 time
-
codecov-io
Jun 8, 2017
-
Jun 8, 2017
Codecov Report
@@ Coverage Diff @@
## master #3444 +/- ##
==========================================
- Coverage 77.26% 77.23% -0.03%
==========================================
Files 373 373
Lines 25687 25713 +26
==========================================
+ Hits 19846 19860 +14
- Misses 4097 4103 +6
- Partials 1744 1750 +6
Continue to review full report at Codecov.
|
| + return strings.TrimRight(s, "\n") | ||
| +} | ||
| + | ||
| +func ComposeBaseDeclaration(ifaces []interfaces.Interface) ([]byte, error) { |
pedronis
Jun 8, 2017
Contributor
why public and without doc? it could be private and then tested using export_test?
zyga
Jun 8, 2017
Contributor
Ah, indeed. I don't need this to be public actually. I'll make it private
Some checks were not successful
jdstrand
reviewed
Jun 8, 2017
•
"Whenever a new interface is created the developer has to edit a number of files. With the recent improvements those have shrinked to, almost, just the interface module. This patch reduces this to exactly just the new interface module. Now each interface can augment the base declaration straight from the module file."
This PR didn't do what I expected from the quoted description. I thought this PR was going to move the interface specific parts out of basedeclaration.go into the interfaces files themselves (eg, docker-support in baseDeclarationPlugs to interfaces/builtin/docker_support.go and account-control in baseDeclarationSlots to interfaces/builtin/account_control.go and then compose baseDeclarationPlugs and baseDeclarationSlots from the interfaces.
Instead it only split the headers, plugs and slots and you compose the declaration from those. This change alone is fine from my perspective, but it doesn't achieve the goal of "exactly just the new interface module".
|
@jdstrand I have that in another branch, I didn't want to make the review too hard. With https://github.com/zyga/snapd/tree/feature/metadata-defines-base-policy it will be exactly as you had hoped. |
All checks have passed
|
@zyga that's ok there's another branch that refines this but then the description of this one that will go in the commit is very confusing |
zyga commentedJun 8, 2017
•
Edited 1 time
-
zyga
Jun 9, 2017
Whenever a new interface is created the developer has to edit a number
of files. With the recent improvements those have shrinked to, almost,
just the interface module. This patch reduces this to exactly just the
new interface module. Now each interface can augment the base
declaration straight from the module file.
The split change nothing in how the policy looks like but sets the stage
for composing it out of meta-data coming from individual interfaces.
Upcoming branch will move the base declaration into separate interface files.
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com