Merge ~jslarraz/review-tools:fix-custom-device-info into review-tools:master

Proposed by Jorge Sancho Larraz
Status: Merged
Merge reported by: Jorge Sancho Larraz
Merged at revision: f7fd05ad122c55eccc5f3db924160567868b9420
Proposed branch: ~jslarraz/review-tools:fix-custom-device-info
Merge into: review-tools:master
Diff against target: 25 lines (+1/-2)
2 files modified
reviewtools/data/snapd-base-declaration.yaml (+1/-1)
reviewtools/sr_common.py (+0/-1)
Reviewer Review Type Date Requested Status
Alex Murray Approve
Review via email: mp+460395@code.launchpad.net

Commit message

correct custom-device base declaration and attributes

To post a comment you must log in.
Revision history for this message
Jorge Sancho Larraz (jslarraz) wrote :
Revision history for this message
Alex Murray (alexmurray) wrote :

Thanks for this @jslarraz - but I still see the read-devices slot attribute referenced in upstream snapd - https://github.com/snapcore/snapd/blob/master/interfaces/builtin/custom_device.go#L246 - so I think this should be retained - but the change from content to custom-device for the plug attribute looks right.

review: Needs Fixing
f7fd05a... by Jorge Sancho Larraz

retain devices-read/slot

Revision history for this message
Jorge Sancho Larraz (jslarraz) wrote :

Thanks Alex for the review. devices-read/slot retained in commit https://git.launchpad.net/~jslarraz/review-tools/commit/?id=f7fd05ad122c55eccc5f3db924160567868b9420

Whilst there a reference to devices-read in https://snapcraft.io/docs/custom-device-interface, it is not listed in the attributes section. Is is something we want/can address?

Revision history for this message
Alex Murray (alexmurray) wrote :

Thanks @jslarraz - we should reach out to ~morrisong about that documentation issue - or perhaps just add a comment on the associated forum page for that documentation https://forum.snapcraft.io/t/the-custom-device-interface/29487

review: Approve
Revision history for this message
Jorge Sancho Larraz (jslarraz) wrote :

Thanks Alex, I created a commentary in the related forum topic

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/reviewtools/data/snapd-base-declaration.yaml b/reviewtools/data/snapd-base-declaration.yaml
index b3729e2..20c78eb 100644
--- a/reviewtools/data/snapd-base-declaration.yaml
+++ b/reviewtools/data/snapd-base-declaration.yaml
@@ -316,7 +316,7 @@
316 allow-installation: false316 allow-installation: false
317 allow-connection:317 allow-connection:
318 plug-attributes:318 plug-attributes:
319 content: $SLOT(custom-device)319 custom-device: $SLOT(custom-device)
320 deny-auto-connection: true320 deny-auto-connection: true
321 daemon-notify:321 daemon-notify:
322 allow-installation:322 allow-installation:
diff --git a/reviewtools/sr_common.py b/reviewtools/sr_common.py
index c6db26c..49890be 100644
--- a/reviewtools/sr_common.py
+++ b/reviewtools/sr_common.py
@@ -242,7 +242,6 @@ class SnapReview(Review):
242 },242 },
243 "cups": {"cups-socket-directory/slots": ""},243 "cups": {"cups-socket-directory/slots": ""},
244 "custom-device": {244 "custom-device": {
245 "content/plugs": "",
246 "devices/slots": [],245 "devices/slots": [],
247 "read-devices/slots": [],246 "read-devices/slots": [],
248 "files/slots": {},247 "files/slots": {},

Subscribers

People subscribed via source and target branches