Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
Summary: Merge Review: comps-extras Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: katzj@redhat.com
Fedora Merge Review: comps-extras
http://cvs.fedora.redhat.com/viewcvs/devel/comps-extras/ Initial Owner: katzj@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: comps-extras
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |katzj@redhat.com Flag| |fedora-review-
------- Additional Comments From roozbeh@farsiweb.info 2007-02-05 10:26 EST ------- 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/
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: comps-extras
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
katzj@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review- |fedora-review?
------- Additional Comments From katzj@redhat.com 2007-02-05 16:38 EST ------- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: comps-extras
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|katzj@redhat.com |roozbeh@farsiweb.info
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: comps-extras
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
roozbeh@farsiweb.info changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|roozbeh@farsiweb.info |katzj@redhat.com Flag|fedora-review? |fedora-review-
------- Additional Comments From roozbeh@farsiweb.info 2007-02-06 08:59 EST ------- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: comps-extras
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225652
katzj@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|katzj@redhat.com |roozbeh@farsiweb.info Flag|fedora-review- |fedora-review?
------- Additional Comments From katzj@redhat.com 2007-02-07 13:33 EST ------- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225652
Jeremy Katz katzj@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?
--- Comment #5 from Jeremy Katz katzj@redhat.com 2009-03-24 14:24:10 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225652
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED CC| |limburgher@gmail.com AssignedTo|nobody@fedoraproject.org |limburgher@gmail.com Resolution| |RAWHIDE Flag|fedora-review?, needinfo? |fedora-review+ Last Closed| |2012-04-05 10:13:09
--- Comment #6 from Jon Ciesla limburgher@gmail.com 2012-04-05 10:13:09 EDT --- 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.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla