Bug 435572 - Review Request: gnome-hearts - Hearts game for GNOME
Summary: Review Request: gnome-hearts - Hearts game for GNOME
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 291741 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-03-01 23:12 UTC by Christopher Aillon
Modified: 2008-06-03 14:50 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-06-03 14:50:25 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Christopher Aillon 2008-03-01 23:12:19 UTC
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 23:14:38 UTC
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 16:01:50 UTC
*** Bug 291741 has been marked as a duplicate of this bug. ***

Comment 3 Richi Plana 2008-03-03 02:16:52 UTC
Pardonez moi. First baby threw a wrench in the gears. Glad to hear it's coming,
though.

Comment 4 Mamoru TASAKA 2008-03-03 14:20:10 UTC
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 16:39:56 UTC
(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 16:50:16 UTC
(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 22:18:16 UTC
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-10 00:26:57 UTC
Then ask for fedora-packaing. Now this is a must.

Comment 9 Christopher Aillon 2008-03-12 06:20:04 UTC
I have no idea what that means.

Comment 10 Mamoru TASAKA 2008-03-12 07:40:22 UTC
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 22:13:01 UTC
Okay, are those the only things wrong with the spec?

Comment 12 Mamoru TASAKA 2008-03-16 03:23:24 UTC
(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 15:25:58 UTC
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 15:36:20 UTC
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 15:41:38 UTC
Does it mean that you want me to wait for FPC voting?

Comment 16 Christopher Aillon 2008-04-20 17:43:46 UTC
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 18:37:52 UTC
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 18:57:54 UTC
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 18:58:49 UTC
(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 19:15:00 UTC
It's current.

Comment 21 Mamoru TASAKA 2008-05-11 17:23:11 UTC
- 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 17:00:01 UTC
ping?

Comment 23 Mamoru TASAKA 2008-05-28 16:58:45 UTC
ping again?

Comment 24 Christopher Aillon 2008-05-28 18:56:52 UTC
Okay, I'll add COPYING-DOCS and I'll investigate %check.

Comment 25 Christopher Aillon 2008-05-28 19:00:47 UTC
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 19:24:10 UTC
cvs done.

Comment 27 Christopher Aillon 2008-06-03 14:50:25 UTC
packages built, closing out.


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