-
Notifications
You must be signed in to change notification settings - Fork 562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
o/ifacestate: properly undo setup-profiles on component installation #13977
o/ifacestate: properly undo setup-profiles on component installation #13977
Conversation
2a90788
to
989ca8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems a strange approach unlike any other undo logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, small comment
return err | ||
} | ||
|
||
appSet, err := appSetForTask(task, snapInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe worth adding a comment here, about what is included in the appset
return err | ||
} | ||
|
||
appSet, err := appSetForSnapRevision(st, snapInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
6c2cda2
to
89a5fad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
This allows us to pass in a different app sets in doSetupProfiles and in undoSetupProfiles. The latter will not have components that were just set up in it.
89a5fad
to
3766d45
Compare
This corrects and tests the behavior of undoing a
setup-profiles
task that contains components. We changesetupProfilesForSnap
to besetupProfilesForAppSet
, so we can pass in aSnapAppSet
that doesn't contain the components being set up for the current task when undoing the task.