Fedora Merge Review: comps-extras http://cvs.fedora.redhat.com/viewcvs/devel/comps-extras/ Initial Owner: katzj
rpmlint output: W: comps-extras no-url-tag W: comps-extras no-documentation (Assumed fine) Random notes: * Consider changing the buildroot to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * URL should be provided for upstream tarball at least, to check against (MUST item) * GPL may not be a very appropriate license for a set of PNG images * Change make to "make %{?_smp_mflags}" * Change %defattr(-,root,root) to %defattr(-,root,root,-) * Packages puts files in /usr/share/pixmaps without owning that directory or depending on any packages that owns it (blocker). * Better add extra slash at the end of %{_datadir}/pixmaps/comps to show it's a directory: %{_datadir}/pixmaps/comps/
(In reply to comment #1) > rpmlint output: > W: comps-extras no-url-tag Yep, there's not one. > W: comps-extras no-documentation And there isn't any > Random notes: > * Consider changing the buildroot to > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Sure, done. > * URL should be provided for upstream tarball at least, to check against (MUST item) There isn't an upstream tarball location. The upstream _are_ the packages that are built. > * GPL may not be a very appropriate license for a set of PNG images It's not normal, but it's fine. > * Change make to "make %{?_smp_mflags}" Given that there's nothing actually done, this doesn't actually make a difference > * Change %defattr(-,root,root) to %defattr(-,root,root,-) Sure > * Packages puts files in /usr/share/pixmaps without owning that directory or > depending on any packages that owns it (blocker). /usr/share/pixmaps is owned by the filesystem package. I'm pretty sure we don't have things requiring it > * Better add extra slash at the end of %{_datadir}/pixmaps/comps to show it's a > directory: %{_datadir}/pixmaps/comps/ One better; added the directory as %dir and then the files underneath.
(In reply to comment #2) > > W: comps-extras no-url-tag > > Yep, there's not one. Use http://www.fedoraproject.org/ then. > There isn't an upstream tarball location. The upstream _are_ the packages > that are built. If there's a source control system with public anonymous access, please point to that. Checking the included tarball against the upstream tarballs are a MUST item in the review list. (BLOCKER) > > * GPL may not be a very appropriate license for a set of PNG images > > It's not normal, but it's fine. Then please include a copy of the GPL license in the source tarball (and , and add a note somewhere in the tarball or the comment field of the image files themselves that the files are licensed under the GPL. Presently, the only mention of the license is the spec file, which means that one cannot confirm that it is used correctly. If there is no mention of free software license somewhere, one should assume that it's proprietary, at least according to the US law. (BLOCKER) > Given that there's nothing actually done, this doesn't actually make a > difference Agreed. But then please remove the line "make" from the %build section. The section is allowed to be empty. > One better; added the directory as %dir and then the files underneath. Choice of style really, but keeping two copies of the same info in two different places is not what I would personally recommend.
(In reply to comment #3) > (In reply to comment #2) > > > W: comps-extras no-url-tag > > Yep, there's not one. > Use http://www.fedoraproject.org/ then. It's used by more than Fedora, though, so that's not appropriate either. And no cvsweb (since that would be a good answer) > > There isn't an upstream tarball location. The upstream _are_ the packages > > that are built. > If there's a source control system with public anonymous access, please point to > that. Checking the included tarball against the upstream tarballs are a MUST > item in the review list. (BLOCKER) Added a comment pointing to the upstream CVS. There aren't tarballs, so that's the best there can be. > > > * GPL may not be a very appropriate license for a set of PNG images > > It's not normal, but it's fine. > Then please include a copy of the GPL license in the source tarball (and , and > add a note somewhere in the tarball or the comment field of the image files > themselves that the files are licensed under the GPL. Presently, the only > mention of the license is the spec file, which means that one cannot confirm > that it is used correctly. > If there is no mention of free software license somewhere, one should assume > that it's proprietary, at least according to the US law. (BLOCKER) There is a mention somewhere -- the spec file is in the upstream source and says GPL. I've dropped COPYING into the CVS repo for the next time there's a pull done. > > Given that there's nothing actually done, this doesn't actually make a > > difference > Agreed. But then please remove the line "make" from the %build section. The > section is allowed to be empty. I don't care enough to keep arguing why this doesn't matter ;) Removed.
Is there anything else I need to do on this one or can we close it out? I think that it should be in good shape now
Fresh review. Good: - rpmlint checks return: comps-extras.src: W: invalid-license LGPL+ Should be LGPLv2+ - package meets naming guidelines - package meets packaging guidelines - license ( ) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file I've fixed in rawhide and rebuilt, APPROVED, closing.