Bug 768700 - Review Request: sugar-flip - Simple strategic game of flipping coins
Summary: Review Request: sugar-flip - Simple strategic game of flipping coins
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Ankur Sinha (FranciscoD)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-12-18 03:49 UTC by Kalpa Welivitigoda
Modified: 2012-04-12 03:25 UTC (History)
4 users (show)

Fixed In Version: sugar-flip-3-2.fc17
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-09 01:59:16 UTC
sanjay.ankur: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
Review notes in a text file (5.14 KB, application/octet-stream)
2012-03-25 10:12 UTC, Ankur Sinha (FranciscoD)
no flags Details

Description Kalpa Welivitigoda 2011-12-18 03:49:48 UTC
Spec URL: http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip.spec
SRPM URL: http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip-1-1.fc16.src.rpm
Description: 

Hi I just finished packaging sugar-flip. I highly appreciate a review.

sugar-flip is a simple strategic game for sugar learning environment where you have to flip coins until they are all heads up. Each time you win, the challenge gets more difficult. You can play flips with your friends over the net.

Comment 1 Ankur Sinha (FranciscoD) 2012-03-24 11:29:04 UTC
Shall have this done by tomorrow max. I'm really sorry for the delay :/

Comment 2 Ankur Sinha (FranciscoD) 2012-03-25 10:11:52 UTC
[+] OK
[-] NA
[?] Issue

[+] Package meets naming and packaging guidelines
[+] Spec file matches base package name.
[+] Spec has consistant macro usage.
[+] Meets Packaging Guidelines.
[+] License
[?] License field in spec matches
Looks like it should also include MIT/X11/BSD and not just GPLv3+?
[ankur@ankur Flip-1]$ find . -name '*' -exec licensecheck '{}' \; | sed
'/UNKNOWN/ d'| sort | uniq
./FlipActivity.py: GPL (v3 or later)
./game.py: GPL (v3 or later)
./sprites.py: MIT/X11 (BSD like)
./toolbar_utils.py: GPL (v3 or later)
./utils.py: GPL (v3 or later)
[ankur@ankur Flip-1]$

[?] License file included in package
MIT/X11/BSD license is not included. Not a blocker though.

[+] Spec in American English
[+] Spec is legible.
[+] Sources match upstream md5sum:
[ankur@ankur SOURCES]$ review-md5check.sh ../SPECS/sugar-flip.spec
Getting http://download.sugarlabs.org/sources/honey/Flip/Flip-1.tar.bz2 to
/tmp/review/Flip-1.tar.bz2
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   262  100   262    0     0    182      0  0:00:01  0:00:01 --:--:--   853
100 23638  100 23638    0     0   5787      0  0:00:04  0:00:04 --:--:-- 13496
11b04aa7d6248b19f1447ff18ebb5f17  /tmp/review/Flip-1.tar.bz2
11b04aa7d6248b19f1447ff18ebb5f17  /home/ankur/rpmbuild/SOURCES/Flip-1.tar.bz2

[-] Package needs ExcludeArch
[?] BuildRequires correct
According to http://fedoraproject.org/wiki/Packaging:Python#BuildRequires the
BR needs to be python{2,3}-devel. Please correct it

[?] Spec handles locales/find_lang
I see gettext included, but I don't see find_lang used in here. 
The generated rpm does not contain any translations as well:

[ankur@ankur noarch]$ rpm -pql sugar-flip-1-1.fc18.noarch.rpm
/usr/share/doc/sugar-flip-1
/usr/share/doc/sugar-flip-1/COPYING
/usr/share/doc/sugar-flip-1/NEWS
/usr/share/sugar/activities/Flip.activity
/usr/share/sugar/activities/Flip.activity/COPYING
/usr/share/sugar/activities/Flip.activity/FlipActivity.py
/usr/share/sugar/activities/Flip.activity/FlipActivity.pyc
/usr/share/sugar/activities/Flip.activity/FlipActivity.pyo
/usr/share/sugar/activities/Flip.activity/NEWS
/usr/share/sugar/activities/Flip.activity/activity
/usr/share/sugar/activities/Flip.activity/activity/activity-flip.svg
/usr/share/sugar/activities/Flip.activity/activity/activity.info
/usr/share/sugar/activities/Flip.activity/game.py
/usr/share/sugar/activities/Flip.activity/game.pyc
/usr/share/sugar/activities/Flip.activity/game.pyo
/usr/share/sugar/activities/Flip.activity/icons
/usr/share/sugar/activities/Flip.activity/icons/new-game.svg
/usr/share/sugar/activities/Flip.activity/setup.py
/usr/share/sugar/activities/Flip.activity/setup.pyc
/usr/share/sugar/activities/Flip.activity/setup.pyo
/usr/share/sugar/activities/Flip.activity/sprites.py
/usr/share/sugar/activities/Flip.activity/sprites.pyc
/usr/share/sugar/activities/Flip.activity/sprites.pyo
/usr/share/sugar/activities/Flip.activity/toolbar_utils.py
/usr/share/sugar/activities/Flip.activity/toolbar_utils.pyc
/usr/share/sugar/activities/Flip.activity/toolbar_utils.pyo
/usr/share/sugar/activities/Flip.activity/utils.py
/usr/share/sugar/activities/Flip.activity/utils.pyc
/usr/share/sugar/activities/Flip.activity/utils.pyo
[ankur@ankur noarch]$

[-] Package is relocatable and has a reason to be.
[+] Package has %defattr and permissions on files is good.
defattr not required anymore

[+] Package is code or permissible content.
[-] Doc subpackage needed/used.
[+] Packages %doc files don't affect runtime.

[?] Package is a GUI app and has a .desktop file
This does feel like a GUI app. Please include a desktop file if this is so.

[+] Package compiles and builds on at least one arch.
Scratch build for rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3930563

[?] Package has no duplicate files in %files.
While there aren't duplicate files, are the setup.py? required? This is only
required for building the package. It shouldn't be included in the rpm IMO.

[+] Package doesn't own any directories other packages own.
[+] Package owns all the directories it creates.
[?] No rpmlint output.
[ankur@ankur noarch]$ rpmlint sugar-flip-1-1.fc18.noarch.rpm
../../SPECS/sugar-flip.spec ../../SRPMS/sugar-flip-1-1.fc16.src.rpm 
sugar-flip.noarch: W: non-standard-group Sugar/Activities
../../SPECS/sugar-flip.spec:6: W: non-standard-group Sugar/Activities
sugar-flip.src: W: non-standard-group Sugar/Activities
2 packages and 1 specfiles checked; 0 errors, 3 warnings.
[ankur@ankur noarch]$

Not anything serious. Please recheck if the group you've used is a standard
one.

[+] final provides and requires are sane: Looks okay
== sugar-flip-1-1.fc18.noarch.rpm ==
Provides:
sugar-flip = 1-1.fc18

Requires:
/usr/bin/env
sugar

[ankur@ankur temp]$

SHOULD Items:

[+] Should build in mock.
[+] Should build on all supported archs
[?] Should function as described.
Please verify this

[+] Should have dist tag

Issues:

[?] License field in spec matches
[?] License file included in package
[?] BuildRequires correct
[?] Spec handles locales/find_lang
[?] Package is a GUI app and has a .desktop file
[?] Package has no duplicate files in %files.
[?] No rpmlint output.

Comment 3 Ankur Sinha (FranciscoD) 2012-03-25 10:12:56 UTC
Created attachment 572507 [details]
Review notes in a text file

Comment 4 Kalpa Welivitigoda 2012-03-30 16:33:21 UTC
Hi Ankur thanks for the review. Here are the new files, hope now it is good to go.

This runs in sugar-emulator so there's no need of a desktop file.

Group, although not standard it is the one for sugar activities.

Sugar packaging guide doesn't say about excluding setup.py. Perhaps if it is there children who use the activity can see it's source and learn. That may be the reason.

Spec URL: http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip.spec
SRPM URL:
http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip-1-2.fc16.src.rpm

rpmlint sugar-flip.spec ../SRPMS/sugar-flip-1-2.fc16.src.rpm ../RPMS/noarch/sugar-flip-1-2.fc16.noarch.rpm 
sugar-flip.spec:6: W: non-standard-group Sugar/Activities
sugar-flip.src: W: non-standard-group Sugar/Activities
sugar-flip.noarch: W: non-standard-group Sugar/Activities
2 packages and 1 specfiles checked; 0 errors, 3 warnings.

Comment 5 Ankur Sinha (FranciscoD) 2012-03-30 17:14:15 UTC
hello Kalpa,

I just confirmed. The setup.py file isn't supposed to install itself. Please remove it from the rpm. It's an extraneous file and should not be included. If one wants to see how the package works, they're supposed to download the source, either from the source rpm or the source itself. 

The rest looks good to me. Please just remove the setup.py* files, and I will approve the package :)

Thanks,
Ankur

Comment 6 Kalpa Welivitigoda 2012-03-30 18:24:24 UTC
Hi Ankur,

setup.py removed.

Spec URL: http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip.spec
SRPM URL:
http://callkalpa.fedorapeople.org/sugar-flip/sugar-flip-1-3.fc16.src.rpm

rpmlint sugar-flip.spec ../SRPMS/sugar-flip-1-3.fc16.src.rpm ../RPMS/noarch/sugar-flip-1-3.fc16.noarch.rpm 
sugar-flip.spec:6: W: non-standard-group Sugar/Activities
sugar-flip.src: W: non-standard-group Sugar/Activities
sugar-flip.noarch: W: non-standard-group Sugar/Activities
2 packages and 1 specfiles checked; 0 errors, 3 warnings.

Comment 7 Ankur Sinha (FranciscoD) 2012-03-30 19:03:24 UTC
http://koji.fedoraproject.org/koji/taskinfo?taskID=3949499

XXX APPROVED XXX

Comment 8 Kalpa Welivitigoda 2012-03-30 19:16:52 UTC
New Package SCM Request
=======================
Package Name: sugar-flip
Short Description: Simple strategic game for sugar
Owners: callkalpa
Branches: f15 f16 f17
InitialCC: pbrobinson

Comment 9 Gwyn Ciesla 2012-03-30 19:25:05 UTC
Git done (by process-git-requests).

Comment 10 Fedora Update System 2012-03-31 04:18:48 UTC
sugar-flip-3-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/sugar-flip-3-2.fc16

Comment 11 Fedora Update System 2012-03-31 04:18:59 UTC
sugar-flip-3-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/sugar-flip-3-2.fc15

Comment 12 Fedora Update System 2012-03-31 04:19:08 UTC
sugar-flip-3-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/sugar-flip-3-2.fc17

Comment 13 Fedora Update System 2012-03-31 18:47:59 UTC
sugar-flip-3-2.fc17 has been pushed to the Fedora 17 testing repository.

Comment 14 Fedora Update System 2012-04-09 01:59:16 UTC
sugar-flip-3-2.fc16 has been pushed to the Fedora 16 stable repository.

Comment 15 Fedora Update System 2012-04-09 01:59:27 UTC
sugar-flip-3-2.fc15 has been pushed to the Fedora 15 stable repository.

Comment 16 Fedora Update System 2012-04-12 03:25:01 UTC
sugar-flip-3-2.fc17 has been pushed to the Fedora 17 stable repository.


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