Bug 841239

Summary: Review Request: sugar-story - an activity that uses images to prompt the learner to tell stories
Product: [Fedora] Fedora Reporter: Danishka Navin <danishka>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: callkalpa, notting, package-review, panemade, pbrobinson
Target Milestone: ---Flags: panemade: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: sugar-story-9-6.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-06-16 02:11:12 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Danishka Navin 2012-07-18 09:42:48 EDT
Spec URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story.spec

SRPM URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story-5-1.fc17.src.rpm


Description: 

Story is a fun way to get children to engage in narration. Simply generate some images and let the children's imagination do the rest.

Extending Story

Ask the students to transcribe and refine their stories using the Sugar Write activity. (They can save to the Journal the images used to prompt their story by typing Alt-1 and then embed this image into their Write document.)

Story can be shared, so the narration can be created amongst multiple students. They can chain together a story by taking turns; each turn involving generating narration for one image. 

http://wiki.sugarlabs.org/go/Activities/Story


Fedora Account System Username: snavin
Comment 1 Kalpa Welivitigoda 2013-01-07 11:37:06 EST
I did a koji scratch build and tested the activity in sugar-emulator. Works fine.

koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4845850
Comment 2 Kalpa Welivitigoda 2013-06-03 08:50:53 EDT
Version 9 of Story is in upstream. Shall we have the spec and srpm for the latest version?
Comment 4 Kalpa Welivitigoda 2013-06-04 08:37:09 EDT
Can we have python2-devel for BuildRequires
Double entry of CREDIT in %doc, add COPYING there
License is GPLv3
Pay attention to Rpmlint output below

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package contains BR: python2-devel or python3-devel
  See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires
- If (and only if) the source package includes the text of the license(s) in
  its own file, then that file, containing the text of the license(s) for the
  package is included in %doc.
  Note: Cannot find COPYING in rpm(s)
  See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text


===== MUST items =====

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package requires other packages for directories it uses.
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package is not known to require ExcludeArch.
[ ]: Package complies to the Packaging Guidelines
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "MIT/X11 (BSD like)", "GPL (v3 or later)", "Unknown or generated". 1
     files have unknown license. Detailed output of licensecheck in
     /home/kalpa/fedora-review/841239-sugar-story/licensecheck.txt
[ ]: The spec file handles locales properly.
[ ]: Package consistently uses macro is (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Package must own all directories that it creates.
[ ]: Package does not own files or directories owned by other packages.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 10240 bytes in 2 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Python:
[ ]: Python eggs must not download any dependencies during the build process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: sugar-story-9-1.fc18.noarch.rpm
sugar-story.noarch: E: invalid-lc-messages-dir /usr/share/locale/nah/LC_MESSAGES/org.sugarlabs.StoryActivity.mo
1 packages and 0 specfiles checked; 1 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint sugar-story
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
sugar-story (rpmlib, GLIBC filtered):
    /usr/bin/env
    sugar



Provides
--------
sugar-story:
    sugar-story



Source checksums
----------------
http://download.sugarlabs.org/sources/honey/Story/Story-9.tar.bz2 :
  CHECKSUM(SHA256) this package     : 105bc01477d3dbfc157f0e49c913106f8a7f70d4f82aea5db64875b89b678fa9
  CHECKSUM(SHA256) upstream package : 105bc01477d3dbfc157f0e49c913106f8a7f70d4f82aea5db64875b89b678fa9


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-18-x86_64
Command line :/usr/bin/fedora-review -vb 841239
Comment 5 Kalpa Welivitigoda 2013-06-04 08:51:14 EDT
rm po/nah.po
in %prep will solve the "/usr/share/locale/nah/LC_MESSAGES/org.sugarlabs.StoryActivity.mo" issue
Comment 6 Danishka Navin 2013-06-04 22:44:12 EDT
changed the spec as per comments #4 and #5

SPEC file URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story.spec

SRPM URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story-9-2.fc18.src.rpm
Comment 7 Kalpa Welivitigoda 2013-06-05 03:55:44 EDT
License should be GPLv3 I guess
Comment 9 Parag AN(पराग) 2013-06-06 02:10:07 EDT
Danishka, here is a official review

+ Package builds in mock successfully

- rpmlint on rpms gave
sugar-story.noarch: W: incoherent-version-in-changelog 9-3 ['9-4.fc19', '9-4']
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ Source verified with upstream as (sha256sum)
srpm tarball: 105bc01477d3dbfc157f0e49c913106f8a7f70d4f82aea5db64875b89b678fa9
upstream tarball:105bc01477d3dbfc157f0e49c913106f8a7f70d4f82aea5db64875b89b678fa9

- License tag is GPLv3 which is not completely valid.

+ Rest looks as per packaging guidelines.


Suggestions:
1) Remove the following line from %prep. I see latest upstream release nah.po is fixed.
  rm po/nah.po

2) License tag should be GPLv3+ and MIT. 
Read https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses

Also, Add a comment above license tag 
# grecord.py and sprites.py is in MIT and all other files in GPLv3+

3) You may want to have release tag number 3 and not 4 for last update. But now when you will fix above issue 1 by adding changelog, rpmlint warning will go away.
Comment 10 Parag AN(पराग) 2013-06-06 02:25:14 EDT
4) Don't use any backslash between %{buildroot} and %{_prefix}, so your %install should look like
%{__python} ./setup.py install --prefix=%{buildroot}%{_prefix}
Comment 11 Danishka Navin 2013-06-06 03:05:33 EDT
Thanks Parag for the review and comments.

Revert the spec file as per comments #9 and #10

SPEC URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story.spec

SRPM URL: http://snavin.fedorapeople.org/packages/sugar-story/sugar-story-9-5.fc18.src.rpm
Comment 12 Parag AN(पराग) 2013-06-06 03:19:48 EDT
You missed to update license tag. Read Suggestion 2 again. You need to set it
GPLv3+ and MIT.
Comment 14 Parag AN(पराग) 2013-06-06 05:26:07 EDT
I see spelling mistake in latest changelog. Please change tab to tag. Otherwise recent srpm Looks good now.


APPROVED.
Comment 16 Danishka Navin 2013-06-06 05:53:02 EDT
New Package SCM Request
=======================
Package Name: sugar-story
Short Description: An activity that uses images to prompt the learner to tell stories
Owners: snavin
Branches: f17 f18 f19
InitialCC:
Comment 17 Jon Ciesla 2013-06-06 07:47:11 EDT
Git done (by process-git-requests).
Comment 18 Danishka Navin 2013-06-07 04:35:35 EDT
I got following error but got the mail with the suject of "Package: sugar-story-9-6.fc17 Tag: f17-updates-candidate Status: complete Built by: snavin"



$fedpkg build
Building sugar-story-9-6.fc17 for f17-candidate
Created task: 5478884
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=5478884
Watching tasks (this may be safely interrupted)...
5478884 build (f17-candidate, /sugar-story:cf3e7ef8b07f4c626bfb1fe3b748241372cdaf47): open (buildvm-18.phx2.fedoraproject.org)
  5478885 buildSRPMFromSCM (/sugar-story:cf3e7ef8b07f4c626bfb1fe3b748241372cdaf47): open (buildvm-09.phx2.fedoraproject.org)
  5478885 buildSRPMFromSCM (/sugar-story:cf3e7ef8b07f4c626bfb1fe3b748241372cdaf47): open (buildvm-09.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
Could not execute build: [('SSL routines', 'SSL3_GET_SERVER_CERTIFICATE', 'certificate verify failed')]
Comment 19 Peter Robinson 2013-06-07 08:08:20 EDT
> Could not execute build: [('SSL routines', 'SSL3_GET_SERVER_CERTIFICATE',
> 'certificate verify failed')]

Often indicates a network issue, I get that if my wifi drops out for some reason
Comment 20 Danishka Navin 2013-06-07 08:15:08 EDT
ok then
Comment 21 Fedora Update System 2013-06-08 09:40:42 EDT
sugar-story-9-6.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/sugar-story-9-6.fc18
Comment 22 Fedora Update System 2013-06-08 09:40:57 EDT
sugar-story-9-6.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/sugar-story-9-6.fc19
Comment 23 Fedora Update System 2013-06-08 09:41:08 EDT
sugar-story-9-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/sugar-story-9-6.fc17
Comment 24 Fedora Update System 2013-06-08 14:06:16 EDT
sugar-story-9-6.fc19 has been pushed to the Fedora 19 testing repository.
Comment 25 Fedora Update System 2013-06-16 02:11:12 EDT
sugar-story-9-6.fc19 has been pushed to the Fedora 19 stable repository.
Comment 26 Fedora Update System 2013-06-17 21:33:04 EDT
sugar-story-9-6.fc18 has been pushed to the Fedora 18 stable repository.
Comment 27 Fedora Update System 2013-06-17 21:37:12 EDT
sugar-story-9-6.fc17 has been pushed to the Fedora 17 stable repository.