Red Hat Bugzilla – Bug 1369560
Review Request: snapd-xdg-open - Bridge allowing (untrusted) snap applications to use xdg-open
Last modified: 2017-09-01 13:53:47 EDT
Spec URL: https://fedorapeople.org/~zyga/snapd-xdg-open.spec SRPM URL: https://fedorapeople.org/~zyga/snapd-xdg-open-0.0.0-1.fc24.src.rpm Description: A D-Bus-activated helper service allowing snaps to launch URLs on the host where its installed. The service validates and checks the requested URLs before allowing them to be opened. Fedora Account System Username: zyga
Taking this review.
Initial issues: * Not properly set up for pre-release. It is perfectly acceptable to package up snapshot pre-releases, provided that the guidelines are followed for versioning them: https://fedoraproject.org/wiki/Packaging:Versioning#Pre-Release_packages * URL is invalid. While it does properly redirect, I would strongly prefer to see the correct URL set here, which I understand it to be "https://github.com/snapcore/snapd-xdg-open" * Source0 is invalid. This is a serious problem. Now, since you're currently using snapshots, you should follow the guidelines for snapshot URLs, as mentioned here: https://fedoraproject.org/wiki/Packaging:SourceURL#Commit_Revision A suggestion: You can set the source URL to "https://github.com/snapcore/snapd-xdg-open/archive/%{commit0}/%{name}-%{shortcommit0}.tar.gz", which will work with spectool to produce a proper snapshot. You'll need to add "-n %{name}-%{commit0}" to the %setup invocation, also.
No response in months, marking as DEADREVIEW.
I've updated the spec file and produced a new SRPM with all necessary changes to fit Neals review comments. You can find the new files at * https://mm.gravedo.de/files/snapd-xdg-open.spec * https://mm.gravedo.de/files/snapd-xdg-open-0.0.0-0.1.git6fed357.fc25.src.rpm
@Simon: Please open a new review request and close this one as a duplicate of it.
This has been obsoleted by the re-implementation of this code in snapd[1]. [1]: https://github.com/snapcore/snapd/commit/047538e13d0178e30d7cba551ac5cf41c57b78b5