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

snap: migrate to base: core24 #70

Merged
merged 9 commits into from
May 8, 2024
Merged

snap: migrate to base: core24 #70

merged 9 commits into from
May 8, 2024

Conversation

Saviq
Copy link
Collaborator

@Saviq Saviq commented Jan 29, 2024

No description provided.

@Saviq Saviq force-pushed the core24 branch 4 times, most recently from 48b89b5 to 4ac4042 Compare January 31, 2024 14:03
@github-actions github-actions bot force-pushed the core24 branch 3 times, most recently from c3496a8 to 3f60f8a Compare February 9, 2024 11:49
@Saviq Saviq force-pushed the core24 branch 4 times, most recently from f5a3327 to f874495 Compare April 17, 2024 15:36
@Saviq Saviq force-pushed the core24 branch 3 times, most recently from ae28ea4 to c25e776 Compare April 24, 2024 13:49
@Saviq Saviq added the no-merge label Apr 24, 2024
@Saviq Saviq marked this pull request as ready for review April 24, 2024 14:16
@Saviq Saviq requested a review from a team as a code owner April 24, 2024 14:16
@Saviq Saviq requested a review from AlanGriffiths April 24, 2024 14:16
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Consolidating the plugs & slots does avoid some repetition, but makes it less clear why each is required. That might matter if we come to clear things up. (It's a fairly balanced trade off - I can be persuaded.)

snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/snapcraft.yaml Show resolved Hide resolved
Copy link
Collaborator Author

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

Consolidating the plugs & slots does avoid some repetition, but makes it less clear why each is required. That might matter if we come to clear things up. (It's a fairly balanced trade off - I can be persuaded.)

You mean rather than in "global" plugs:? Yeah, I learned that's not to be used that way… check e.g. mir-test-tools-22-edge:

Warning: implicit plug assignment in 'login-session-control', 'opengl', and 'x11'. Plugs should be assigned to the app to which they apply, and not implicitly assigned via the global 'plugs:' stanza which is intended for configuration only.
(Reference: https://snapcraft.io/docs/snapcraft-interfaces)
Warning: implicit slot assignment in 'wayland'. Slots should be assigned to the app to which they apply, and not implicitly assigned via the global 'slots:' stanza which is intended for configuration only.
(Reference: https://snapcraft.io/docs/snapcraft-interfaces)

snap/snapcraft.yaml Show resolved Hide resolved
snap/snapcraft.yaml Outdated Show resolved Hide resolved
@AlanGriffiths
Copy link
Contributor

You mean rather than in "global" plugs:?

No, I mean shared (by anchors) across all the apps

@Saviq
Copy link
Collaborator Author

Saviq commented Apr 25, 2024

No, I mean shared (by anchors) across all the apps

Ah. Well, if we could stick to global plugs like we had them, yeah - but because that's not The Way™, the only other option is enumerating them all… and yes, on the one hand we only had e.g. pulseaudio apply to the SDL app, not all of them. We might need someone else to convince us one way or the other.

@Saviq Saviq force-pushed the core24 branch 2 times, most recently from c84d214 to a0ce722 Compare May 6, 2024 11:59
@Saviq Saviq removed the no-merge label May 6, 2024
@Saviq Saviq requested a review from AlanGriffiths May 6, 2024 11:59
Saviq added 2 commits May 7, 2024 15:02
Until it fixes complex grammar in stable.
@Saviq
Copy link
Collaborator Author

Saviq commented May 8, 2024

@AlanGriffiths this can land now, ARM builds will fail until canonical/snapcraft#4786 makes its way into stable.

@@ -31,6 +31,7 @@ jobs:
with:
architecture: ${{ matrix.platform }}
snapcraft-token: ${{ secrets.SNAPCRAFT_TOKEN }}
snapcraft-channel: edge
Copy link
Contributor

Choose a reason for hiding this comment

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

Really?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just for a couple of days ;)

Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Just for a couple of days ;)

OK then

@AlanGriffiths AlanGriffiths added this pull request to the merge queue May 8, 2024
Merged via the queue into main with commit 5dc6766 May 8, 2024
2 of 3 checks passed
@AlanGriffiths AlanGriffiths deleted the core24 branch May 8, 2024 11:33
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.

2 participants