Bug 225652 - Merge Review: comps-extras
Merge Review: comps-extras
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jon Ciesla
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 12:51 EST by Nobody's working on this, feel free to take it
Modified: 2012-04-05 10:13 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-04-05 10:13:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 12:51:49 EST
Fedora Merge Review: comps-extras

http://cvs.fedora.redhat.com/viewcvs/devel/comps-extras/
Initial Owner: katzj@redhat.com
Comment 1 Roozbeh Pournader 2007-02-05 10:26:29 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/
Comment 2 Jeremy Katz 2007-02-05 16:38:03 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.

Comment 3 Roozbeh Pournader 2007-02-06 08:59:44 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.
Comment 4 Jeremy Katz 2007-02-07 13:33:27 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.
Comment 5 Jeremy Katz 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
Comment 6 Jon Ciesla 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.

Note You need to log in before you can comment on or make changes to this bug.