Skip to content
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

[sdk-assistant] Hide snapshots in the listing unless told otherwise. JB#58916 #334

Merged
merged 3 commits into from Oct 10, 2022

Conversation

martyone
Copy link
Member

@martyone martyone commented Oct 4, 2022

No description provided.

@@ -308,7 +308,7 @@ build target and using the clone to persist the redefined clean state.

$ sdk-assistant clone SailfishOS-4.2.0.21-{aarch64,MyDevice}
$ sdk-assistant maintain SailfishOS-4.2.0.21-MyDevice
... add/rm packages etc. ...
... add/rm repositories, packages etc. ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually leave out packages here. Adding device-specific repositories to the target makes sense, but I think in most cases adding packages should be done in snapshots, as otherwise it might be difficult to return to the clean state. Sure, sometimes it might make sense to add packages to the target, but that should be the exception and not the rule.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, you just missed that this particular example is exactly about your suggestion :) See that it clones the (very) original target and uses the clone to define a new clean state by preinstalling adaptation-specific packages (from adaptation-specific repos).

Copy link
Member

@vigejolla vigejolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion, otherwise lgtm

The '--snapshot' option nowadays only controls how snapshots are
selected, so this is not the right place for this kind of information
anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants