Bug 435572 - Review Request: gnome-hearts - Hearts game for GNOME
Review Request: gnome-hearts - Hearts game for GNOME
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
: 291741 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-01 18:12 EST by Christopher Aillon
Modified: 2008-06-03 10:50 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-06-03 10:50:25 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Christopher Aillon 2008-03-01 18:12:19 EST
Spec URL: http://caillon.fedorapeople.org/gnome-hearts.spec
SRPM URL: http://caillon.fedorapeople.org/gnome-hearts-0.2.1-1.fc9.src.rpm
Description: 
An implementation of the classic hearts card game for the GNOME desktop,
featuring configurable rulesets and editable computer opponents to satisfy
widely diverging playing styles. Gnome Hearts is Free Software, released
under the GNU General Public License and should be able to run on any
computer that can run the GNOME desktop.
Comment 1 Christopher Aillon 2008-03-01 18:14:38 EST
Bug 291741 was closed due to inactivity.  I took richi's version at 0.2-3.svn163
and did my own package review of it before posting it, and this is the result.
Comment 2 Mamoru TASAKA 2008-03-02 11:01:50 EST
*** Bug 291741 has been marked as a duplicate of this bug. ***
Comment 3 Richi Plana 2008-03-02 21:16:52 EST
Pardonez moi. First baby threw a wrench in the gears. Glad to hear it's coming,
though.
Comment 4 Mamoru TASAKA 2008-03-03 09:20:10 EST
For 0.2.1-1:

* License
  - Please change the license tag to "GPLv2+ aned GFDL".
    * Document files under %{_datadir}/gnome/help/ are licensed
      under GFDL.
    * The rest parts are licensed under GPLv2+
  
* Documents
  - "COPYING" file is rather mandatory for %doc if it
    exists.

* desktop-file-install
  - Please call desktop-file-install for installing desktop file.

* Timestamps
  - I recommend to use
------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
------------------------------------------------------
    to keep timestamps on installed files. This method usually
    works for recent autotool-based Makefiles.
Comment 5 Christopher Aillon 2008-03-07 11:39:56 EST
(In reply to comment #4)
> For 0.2.1-1:
> 
> * License
>   - Please change the license tag to "GPLv2+ aned GFDL".
>     * Document files under %{_datadir}/gnome/help/ are licensed
>       under GFDL.
>     * The rest parts are licensed under GPLv2+
>   
> * Documents
>   - "COPYING" file is rather mandatory for %doc if it
>     exists.

Above two items OK.

> 
> * desktop-file-install
>   - Please call desktop-file-install for installing desktop file.

It is useless here.  The app installs it for us.  We should only use
desktop-file-install for Fedora-managed specs, not for ones installed by the
application.

> 
> * Timestamps
>   - I recommend to use
> ------------------------------------------------------
> make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
> ------------------------------------------------------
>     to keep timestamps on installed files. This method usually
>     works for recent autotool-based Makefiles.

I recommend getting this fixed in autotools, and not tweaking packages.
Comment 6 Mamoru TASAKA 2008-03-07 11:50:16 EST
(In reply to comment #5)

> > * desktop-file-install
> >   - Please call desktop-file-install for installing desktop file.
> 
> It is useless here.  The app installs it for us.  We should only use
> desktop-file-install for Fedora-managed specs, not for ones installed by the
> application.

This is not. desktop-file-utils is used on Fedora packaging not
only for installing desktop files but also for checking if installed
desktop file meets freedesktop standards (i.e. if 
desktop-file-install (--delete-original, for example)
rejects installed desktop files, it must be fixed).
Comment 7 Christopher Aillon 2008-03-09 18:18:16 EDT
If we want it to be validated, then we should run desktop-file-validate and stop
pretending we need to install it.  This way in case the desktop file ever stops
being installed automatically, or gets installed into a different location, we
also fail the build.
Comment 8 Mamoru TASAKA 2008-03-09 20:26:57 EDT
Then ask for fedora-packaing. Now this is a must.
Comment 9 Christopher Aillon 2008-03-12 02:20:04 EDT
I have no idea what that means.
Comment 10 Mamoru TASAKA 2008-03-12 03:40:22 EDT
Just look at the subsection "desktop-file-install usage" of
http://fedoraproject.org/wiki/Packaging/Guidelines

and the MUST item of
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 11 Christopher Aillon 2008-03-15 18:13:01 EDT
Okay, are those the only things wrong with the spec?
Comment 12 Mamoru TASAKA 2008-03-15 23:23:24 EDT
(In reply to comment #11)
> Okay, are those the only things wrong with the spec?

Please also fix the issue below:
(In reply to comment #5)
> (In reply to comment #4)
> > For 0.2.1-1:
> > 
> > * License
> >   - Please change the license tag to "GPLv2+ aned GFDL".
> >     * Document files under %{_datadir}/gnome/help/ are licensed
> >       under GFDL.
> >     * The rest parts are licensed under GPLv2+
> >   
> > * Documents
> >   - "COPYING" file is rather mandatory for %doc if it
> >     exists.
> 
> Above two items OK.
Comment 13 Christopher Aillon 2008-04-20 11:25:58 EDT
For reference, I raised comment 10 with FPC:
https://www.redhat.com/archives/fedora-packaging/2008-April/msg00076.html
Comment 14 Christopher Aillon 2008-04-20 11:36:20 EDT
And fixed the license and copying file in the spec url.  Those are very very
minor issues and i'd prefer to start with a revision of 1 instead having to bump
for that here.  If you absolutely require a bump for some reason, i can create
an srpm with a .fc10 disttag instead of .fc9
Comment 15 Mamoru TASAKA 2008-04-20 11:41:38 EDT
Does it mean that you want me to wait for FPC voting?
Comment 16 Christopher Aillon 2008-04-20 13:43:46 EDT
If you feel you cannot trust me to do the right thing based on the outcome of
FPC, then yes please wait.  If you feel you can trust me to do the right thing,
then please approve with such a statement.
Comment 17 Christopher Aillon 2008-05-08 14:37:52 EDT
FPC passed my proposal on May 6, 2008
<https://www.redhat.com/archives/fedora-devel-list/2008-May/msg00637.html> and
FESCo ratified it on May 8, 2008
<http://bpepple.fedorapeople.org/fesco/FESCo-2008-05-08.html>
Comment 18 Mamoru TASAKA 2008-05-08 14:57:54 EDT
Would you update your srpm anyway as the last time I checked your
srpm is more than 2 months?
Comment 19 Mamoru TASAKA 2008-05-08 14:58:49 EDT
(In reply to comment #18)
> Would you update your srpm anyway as the last time I checked your
> srpm is more than 2 months?

as the last time I checked your srpm was more than 2 months ago? 

Comment 20 Christopher Aillon 2008-05-08 15:15:00 EDT
It's current.
Comment 21 Mamoru TASAKA 2008-05-11 13:23:11 EDT
- Please also add "COPYING-DOCS" to %doc
- Should desktop-file-validate be moved to %check?

Other things seem okay.
--------------------------------------------------------------
      This package (gnome-hearts) is APPROVED by me
--------------------------------------------------------------
Comment 22 Mamoru TASAKA 2008-05-19 13:00:01 EDT
ping?
Comment 23 Mamoru TASAKA 2008-05-28 12:58:45 EDT
ping again?
Comment 24 Christopher Aillon 2008-05-28 14:56:52 EDT
Okay, I'll add COPYING-DOCS and I'll investigate %check.
Comment 25 Christopher Aillon 2008-05-28 15:00:47 EDT
New Package CVS Request
=======================
Package Name: gnome-hearts
Short Description: Hearts game for GNOME
Owners: caillon
Branches: F-8 F-9
Cvsextras Commits: yes
Comment 26 Kevin Fenzi 2008-05-30 15:24:10 EDT
cvs done.
Comment 27 Christopher Aillon 2008-06-03 10:50:25 EDT
packages built, closing out.

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