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

features: add experimental gpio-chardev-interface flag #15014

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

ZeyadYasser
Copy link
Contributor

@ZeyadYasser ZeyadYasser commented Feb 3, 2025

Due to limitations imposed by the kernel support, the gpio-chardev interface (not implemented yet) is considered experimental and needs experimental.gpio-chardev-interface setting to be enabled in snapd, either explicitly at runtime or through the defaults section in gadget.yaml.

Due to limitations imposed by the kernel support, the gpio-chardev interface
is considered experimental and needs experimental.gpio-chardev-support
setting to be enabled in snapd, either explicitly at runtime or through
the defaults section in gadget.yaml.

Signed-off-by: Zeyad Gouda <[email protected]>
Copy link

github-actions bot commented Feb 3, 2025

Thu Feb 6 14:10:46 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13157903441

No spread failures reported

@@ -124,6 +126,8 @@ var featureNames = map[SnapdFeature]string{
ConfdbControl: "confdb-control",

AppArmorPrompting: "apparmor-prompting",

GPIOChardevSupport: "gpio-chardev-support",
Copy link
Contributor

Choose a reason for hiding this comment

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

other options could be gpio-chardev-interface{,-support}, but I'm fine with the current flag name too

@pedronis do yo have any suggestions here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have strong opinions here, but I prefer gpio-chardev-interface.

@@ -124,6 +126,8 @@ var featureNames = map[SnapdFeature]string{
ConfdbControl: "confdb-control",

AppArmorPrompting: "apparmor-prompting",

GPIOChardevSupport: "gpio-chardev-support",
}

// featuresEnabledWhenUnset contains a set of features that are enabled when not explicitly configured.
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a featuresExported which I can't comment on as it's too far from the changed chunks of code, but unless flags cleanup is a problem, I would try to make this future proof and export a flag too, just in case we need it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, updated

@ZeyadYasser ZeyadYasser requested a review from bboozzoo February 5, 2025 11:22
@pedronis
Copy link
Collaborator

pedronis commented Feb 5, 2025 via email

Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@81fd929). Learn more about missing BASE report.
Report is 79 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #15014   +/-   ##
=========================================
  Coverage          ?   78.25%           
=========================================
  Files             ?     1157           
  Lines             ?   154215           
  Branches          ?        0           
=========================================
  Hits              ?   120685           
  Misses            ?    26100           
  Partials          ?     7430           
Flag Coverage Δ
unittests 78.25% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ZeyadYasser ZeyadYasser closed this Feb 5, 2025
@ZeyadYasser ZeyadYasser reopened this Feb 5, 2025
Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

@ZeyadYasser ZeyadYasser changed the title features: add experimental gpio-chardev-support flag features: add experimental gpio-chardev-interface flag Feb 6, 2025
@ernestl ernestl merged commit 72b960e into canonical:master Feb 6, 2025
61 of 66 checks passed
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.

5 participants