Spec URL: https://otaylor.fedorapeople.org/koji-flatpak.spec SRPM URL: https://otaylor.fedorapeople.org/koji-flatpak-0.1-1.src.rpm Description: koji-flatpak adds the ability to build Flatpak containers to Koji. It has plugins for the XMLRPC hub, the builder nodes, and for the Koji command line. Fedora Account System Username: otaylor
Copr build: https://copr.fedorainfracloud.org/coprs/build/6265295 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231215-koji-flatpak/fedora-rawhide-x86_64/06265295-koji-flatpak/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Taking for review. > Release: 1 This is missing the dist tag, 1%{?dist} > BuildRequires: python3-devel > BuildRequires: python3-setuptools A tiny nit: there's something wrong with whitespace and it doesn't align up with what comes before. I think maybe the rest uses tabs and there's a space here. > %description hub > Flatpak plugin for the Koji XMLRPC hub I think the convention is to use a full stop at the end of description sentences. > %files > %license COPYING > %doc README.md I wonder if it would be better to call this koji-flatpak-data or something? It could be a bit confusing if 'dnf install koji-flatpak' results in just a readme and license file. It would also work to have '%license COPYING' duplicated for all the subpackages and just get rid of the -data subpackage altogether. Not sure if it would be better that way or not :) Another question I have is about the plugin subpackage naming. As I understand it, koji-flatpak-hub is a plugin for koji-hub, koji-flatpak-builder is a plugin for koji-builder, and koji-flatpak-cli is a plugin for koji. If I look at the plugins shipped in the koji srpm, e.g. https://koji.fedoraproject.org/koji/buildinfo?buildID=2242604 they seem to use a bit different naming. How about something like this to have a bit of consistency with the existing plugins: koji-hub-plugin-flatpak instead of koji-flatpak-hub koji-builder-plugin-flatpak instead of koji-flatpak-builder python3-koji-cli-plugin-flatpak instead of koji-flatpak-cli What do you think? I really don't want to bikeshed over naming too much, just wanted to point that out quickly. None if this (with maybe the exception of the dist tag) is a review blocker in my opinion.
(In reply to Kalev Lember from comment #2) > Taking for review. > > > > Release: 1 > > This is missing the dist tag, 1%{?dist} Fixed. > > BuildRequires: python3-devel > > BuildRequires: python3-setuptools > > A tiny nit: there's something wrong with whitespace and it doesn't align up > with what comes before. I think maybe the rest uses tabs and there's a space > here. Fixed. > > %description hub > > Flatpak plugin for the Koji XMLRPC hub > > I think the convention is to use a full stop at the end of description > sentences. Fixed. > > %files > > %license COPYING > > %doc README.md > > I wonder if it would be better to call this koji-flatpak-data or something? > It could be a bit confusing if 'dnf install koji-flatpak' results in just a > readme and license file. It would also work to have '%license COPYING' > duplicated for all the subpackages and just get rid of the -data subpackage > altogether. Not sure if it would be better that way or not :) > > > Another question I have is about the plugin subpackage naming. As I > understand it, koji-flatpak-hub is a plugin for koji-hub, > koji-flatpak-builder is a plugin for koji-builder, and koji-flatpak-cli is a > plugin for koji. If I look at the plugins shipped in the koji srpm, e.g. > https://koji.fedoraproject.org/koji/buildinfo?buildID=2242604 they seem to > use a bit different naming. How about something like this to have a bit of > consistency with the existing plugins: > > koji-hub-plugin-flatpak instead of koji-flatpak-hub > koji-builder-plugin-flatpak instead of koji-flatpak-builder > python3-koji-cli-plugin-flatpak instead of koji-flatpak-cli > > What do you think? I really don't want to bikeshed over naming too much, > just wanted to point that out quickly. None if this (with maybe the > exception of the dist tag) is a review blocker in my opinion. There are two sets of Koji plugins packaged in Fedora already: koji-containerbuild koji-containerbuild - LICENSE and docs and Python library koji-containerbuild-hub - hub plugin (just imports code packaged in koji-containerbuild) koji-containerbuild-builder - builder plugin (just imports code packaged in koji-containerbuild) python3-koji-containerbuild-cli - cli plugin (just imports code packaged in koji-containerbuild) koji-osbuild koji-osbuild - LICENSE and README koji-osbuild-hub - hub plugin koji-osbuild-builder - builder plugin koji-osbuild-cli - cli plugin I copied the koji-osbuild setup - koji-containerbuild isn't very different. I thought about skipping the main package entirely, but while duplicating the LICENSE between the package is explicitly allowed by the packaging guidelines, duplicating the README is probably against them. I'd be happy to put the license and README into a koji-flatpak-common subpackage to make it clear that you aren't supposed to install it directly.
New versions: Spec URL: https://otaylor.fedorapeople.org/koji-flatpak.spec SRPM URL: https://otaylor.fedorapeople.org/koji-flatpak-0.1-2.fc38.src.rpm I fixed the review comments, including renaming koji-flatpak => koji-flatpak-common, and also added a missing dependency of the builder plugin on skopeo.
Nice, thanks! Sure, makes sense to do it like that if we already have two packages following the same pattern.
Fedora review koji-flatpak-0.1-2.fc38.src.rpm 2023-08-11 $ rpmlint koji-flatpak-* =================================================================================== rpmlint session starts =================================================================================== rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 5 koji-flatpak-builder.noarch: W: only-non-binary-in-usr-lib koji-flatpak-hub.noarch: W: only-non-binary-in-usr-lib koji-flatpak-builder.noarch: W: no-documentation koji-flatpak-cli.noarch: W: no-documentation koji-flatpak-hub.noarch: W: no-documentation ==================================================== 5 packages and 0 specfiles checked; 0 errors, 5 warnings, 0 badness; has taken 0.3 s ==================================================== + OK ! needs attention + rpmlint output looks good + The package is named according to Fedora packaging guidelines + The spec file name matches the base package name. + The package meets the Packaging Guidelines + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The license field in the spec file matches the actual license + The license text (LGPL) is included in %license + Spec file is written in American English + Spec file is legible + Upstream sources match the sources in the srpm SHA512 (Download/koji-flatpak-0.1.tar.gz) = f62099e1d2910997856272230d49e1ca566b0b762be21173ddd0007397cb57780fea418a969062af4affc42ae8add16ef33017bd3e94a561557878c3ce3393f1 SHA512 (koji-flatpak-0.1.tar.gz) = f62099e1d2910997856272230d49e1ca566b0b762be21173ddd0007397cb57780fea418a969062af4affc42ae8add16ef33017bd3e94a561557878c3ce3393f1 + Package builds in mock n/a ExcludeArch bugs filed + BuildRequires look sane n/a locale handling + Package does not bundle copies of system libraries n/a Package isn't relocatable + Package owns all the directories it creates + No duplicate files in %files + Permissions are properly set + Consistent use of macros + The package must contain code or permissible content n/a Large documentation files should go in -doc subpackage + Files marked %doc should not affect the runtime of application n/a Static libraries should be in -static n/a Development files should be in -devel n/a -devel must require the fully versioned base + Packages should not contain libtool .la files n/a Proper .desktop file handling + Doesn't own files or directories already owned by other packages + Filenames are valid UTF-8 + Package does not depend on deprecated packages APPROVED
The Pagure repository was created at https://src.fedoraproject.org/rpms/koji-flatpak