Bug 666943

Summary: Review Request: ignuit - Memorization aid based on the Leitner flashcard system
Product: [Fedora] Fedora Reporter: Christoph Wickert <christoph.wickert>
Component: Package ReviewAssignee: Martin Gieseking <martin.gieseking>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mail, martin.gieseking, notting, sanjay.ankur
Target Milestone: ---Flags: martin.gieseking: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: ignuit-0.0.16-2.fc14 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-03-03 03:19:33 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Christoph Wickert 2011-01-03 19:32:51 UTC
Spec URL: http://cwickert.fedorapeople.org/review/ignuit.spec
SRPM URL: http://cwickert.fedorapeople.org/review/ignuit-0.0.16-1.fc15.src.rpm
Description: Ignuit is a memorization aid based on the Leitner flashcard system. It has a GNOME look and feel, a good selection of quiz options, and supports UTF-8. Cards can include embedded audio, images, and mathematical formulae (via LaTeX). It can import and export several file formats, including CSV. Ignuit can be used for both long-term learning and cramming.

Comment 1 Fabian Affolter 2011-01-03 20:36:37 UTC
*** Bug 549223 has been marked as a duplicate of this bug. ***

Comment 2 Christoph Wickert 2011-01-03 20:44:49 UTC
I'm sorry, I did not see the duplicate. Do you want to co-maintain the package with me once it is reviewed?

Comment 3 Martin Gieseking 2011-02-17 20:36:47 UTC
Hi Christoph,

the package looks fine. Just three minor notes:

- According to COPYING.extras, some of the icons are licensed under GPLv2 only.
  Thus, the License tag should be "GPLv3+ and GPLv2".

- The macro file texinfo.tex shouldn't be packaged as it's of no use here.

- If you want to maintain ignuit for EPEL < 6 too, you must use scrollkeeper
  rather than rarian (and call scrollkeeper-update in %post/%postun).


$ rpmlint /var/lib/mock/fedora-14-x86_64/result/*.rpm
ignuit.src: W: spelling-error %description -l en_US formulae -> formula, formulate, formulas
ignuit.x86_64: W: spelling-error %description -l en_US formulae -> formula, formulate, formulas
ignuit.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/ignuit.schemas
ignuit.x86_64: W: no-manual-page-for-binary ignuit
ignuit.x86_64: W: dangerous-command-in-%pre rm
ignuit.x86_64: W: dangerous-command-in-%post rm
3 packages and 0 specfiles checked; 0 errors, 6 warnings.

All warnings can be ignored:
- spelling errors are false positive
- GConf schema files must not be tagged with %config
- no manpage available
- rm commands are part of %gconf_schema_FOO

---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - appplication: GPLv3+
    - icons from gnome-icon-theme: GPLv2

[X] MUST: The License field in the package spec file must match the actual license.
    - should be "GPLv3+ and GPLv2" (plus a short comment)

[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum ignuit-0.0.16.tar.gz*
    9edaf56cbae79d5b0b71f581beacaa6f  ignuit-0.0.16.tar.gz
    9edaf56cbae79d5b0b71f581beacaa6f  ignuit-0.0.16.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied.
[+] MUST: The spec file MUST handle locales properly.
[+] MUST: If a package installs files below %{_datadir}/icons, the icon cache must be updated.
[.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), ...
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop file.
[+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

EPEL <= 5 only:
[+] MUST: The spec file must contain a valid BuildRoot field.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.
[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'

[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, ...
[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[.] SHOULD: Your package should contain man pages for binaries/scripts.

Comment 4 Christoph Wickert 2011-02-19 10:27:15 UTC
> - According to COPYING.extras, some of the icons are licensed under GPLv2 only.
>   Thus, the License tag should be "GPLv3+ and GPLv2".

fixed

> - The macro file texinfo.tex shouldn't be packaged as it's of no use here.

removed

> - If you want to maintain ignuit for EPEL < 6 too, you must use scrollkeeper
>   rather than rarian (and call scrollkeeper-update in %post/%postun).

I have started doing this, but it ended in a mess:
- RHEL < 6 doesn't know the gconf macros, so I'd need to use some "%if 0%{?fedora} || 0%{?rhel} >= 6 ... %else ... %endif" voodoo.
- EPEL 4 needs an additional "killall -HUP gconfd-2 > /dev/null || :"
- RHEL < 6 doesn't have %posttrans, I'd need to change the icon-cache scriptets.
In the end the nested conditionals became longer than the current spec and I have no intentions to maintain this in EPEL anyway, so I decided it's not worth the effort.

I also switched from desktop-file-validate to desktop-file-install to remove "Encoding" (obsolete) and "MimeType" (empty) from the desktop file.

SRPM: http://cwickert.fedorapeople.org/review/ignuit-0.0.16-2.fc16.src.rpm
SPEC: http://cwickert.fedorapeople.org/review/ignuit.spec

Comment 5 Martin Gieseking 2011-02-19 11:55:25 UTC
OK, thanks for the additional information. I didn't expect that adapting the spec to EPEL < 6 would become this complex. In this case I'd skipped it too. :)

The package looks good now, and thus is 

--------
APPROVED
--------

Comment 6 Christoph Wickert 2011-02-19 12:53:47 UTC
Thanks for the review!

New Package SCM Request
=======================
Package Name: ignuit
Short Description: Memorization aid based on the Leitner flashcard system
Owners: cwickert fab
Branches: f13 f14 f15
InitialCC:

Comment 7 Jason Tibbitts 2011-02-19 18:42:21 UTC
The requested package name does not match the name in the ticket summary.
Please fix whichever is wrong and re-raise the fedora-cvs flag.

Comment 8 Christoph Wickert 2011-02-19 20:21:33 UTC
Sorry about that, clearly my fault. But I really don't understand why you cannot look into the spec. Takes probably less time than writing that additional comment.

New Package SCM Request
=======================
Package Name: ignuit
Short Description: Memorization aid based on the Leitner flashcard system
Owners: cwickert fab
Branches: f13 f14 f15
InitialCC:

Comment 9 Jason Tibbitts 2011-02-19 20:33:31 UTC
Because the script which processes these tickets does not have access to the
spec file, and because I'm simply not going to guess as to which spelling you
actually want.

Git done (by process-git-requests).

Comment 10 Fedora Update System 2011-02-19 23:21:34 UTC
ignuit-0.0.16-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/ignuit-0.0.16-2.fc15

Comment 11 Fedora Update System 2011-02-19 23:21:50 UTC
ignuit-0.0.16-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/ignuit-0.0.16-2.fc14

Comment 12 Fedora Update System 2011-02-19 23:22:11 UTC
ignuit-0.0.16-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/ignuit-0.0.16-2.fc13

Comment 13 Fedora Update System 2011-02-20 04:12:35 UTC
ignuit-0.0.16-2.fc15 has been pushed to the Fedora 15 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update ignuit'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/ignuit-0.0.16-2.fc15

Comment 14 Fedora Update System 2011-03-03 03:19:22 UTC
ignuit-0.0.16-2.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Fedora Update System 2011-03-05 02:33:29 UTC
ignuit-0.0.16-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2011-03-05 02:34:34 UTC
ignuit-0.0.16-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.