snapstate,many: implement snap install --unaliased #3375

Merged
merged 5 commits into from May 23, 2017

Conversation

Projects
None yet
5 participants
Contributor

pedronis commented May 22, 2017

No description provided.

@pedronis pedronis added this to the 2.27 milestone May 22, 2017

Codecov Report

Merging #3375 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3375      +/-   ##
==========================================
+ Coverage   77.53%   77.54%   +0.01%     
==========================================
  Files         367      367              
  Lines       25156    25167      +11     
==========================================
+ Hits        19504    19516      +12     
+ Misses       3904     3903       -1     
  Partials     1748     1748
Impacted Files Coverage Δ
client/snap_op.go 81.35% <ø> (ø) ⬆️
overlord/snapstate/flags.go 100% <ø> (ø) ⬆️
cmd/snap/cmd_snap_op.go 68.08% <100%> (+0.07%) ⬆️
daemon/api.go 73.01% <100%> (+0.14%) ⬆️
overlord/snapstate/handlers.go 69.84% <100%> (+0.1%) ⬆️
overlord/ifacestate/helpers.go 62.86% <0%> (ø) ⬆️
interfaces/sorting.go 96.66% <0%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2380588...61614f1. Read the comment docs.

The "--unaliased" name sounds a little weird to me. How about '--no-aliases'? Otherwise looks good.

Can you add a test that installs a snap with --unaliased and then refreshes?
Also, could you add Unaliased to TestEnableRunThrough?

+ if err != nil {
+ return snapstate.Flags{}, err
+ }
+ if inst.Unaliased {
@chipaca

chipaca May 23, 2017

Member

is this better, or worse than flags.Unaliased = inst.Unaliased?

Contributor

pedronis commented May 23, 2017

@chipaca

Can you add a test that installs a snap with --unaliased and then refreshes?
that's a pain as spread tests, I might manage with a managers_test.go test

Also, could you add Unaliased to TestEnableRunThrough?
will do

Just a trivial suggestion, but LGTM with or without it.

@@ -797,6 +798,17 @@ func (inst *snapInstruction) modeFlags() (snapstate.Flags, error) {
return modeFlags(inst.DevMode, inst.JailMode, inst.Classic)
}
+func (inst *snapInstruction) installFlags() (snapstate.Flags, error) {
@niemeyer

niemeyer May 23, 2017

Contributor

Do we need a separate method for this? Feels like it might be in modeFlags itself.

pedronis added some commits May 23, 2017

@pedronis pedronis merged commit 27a9d88 into snapcore:master May 23, 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

@pedronis pedronis deleted the pedronis:install-unaliased branch May 24, 2017

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