[Bug 652257] Review Request: focuswriter - A fullscreen, distraction-free writing program

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=652257

Golo Fuchert <packages at golotop.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #5 from Golo Fuchert <packages at golotop.de> 2010-12-20 14:45:38 EST ---
It took me a little longer to review this package because there seem to be some
stability issues. However, a F13 64-bit KDE-spin computer was the only case
where focuswriter crashed reproduceably, so maybe it's not focuswriter's fault.
I will investigate on this further, but because focuswriter seems to run on
most machines and the crash maybe caused by an external library here is the
official review:

$ rpmlint RPMS/x86_64/focuswriter-1.3.1-2.fc13.x86_64.rpm
SRPMS/focuswriter-1.3.1-2.fc13.src.rpm SPECS/focuswriter.spec 
focuswriter.x86_64: W: spelling-error Summary(en_US) fullscreen -> full screen,
full-screen, firescreen
focuswriter.x86_64: W: spelling-error %description -l en_US fullscreen -> full
screen, full-screen, firescreen

- false positives

focuswriter.x86_64: W: no-manual-page-for-binary focuswriter

- maybe something for the future?

focuswriter.src: W: spelling-error Summary(en_US) fullscreen -> full screen,
full-screen, firescreen
focuswriter.src: W: spelling-error %description -l en_US fullscreen -> full
screen, full-screen, firescreen

- same false positives again

2 packages and 1 specfiles checked; 0 errors, 5 warnings.

So nothing serious here.

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

[+] = 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.
    GPLv3+ according to the source file headers
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The file containing the text of the license(s) for the package must
be included in %doc.
    COPYING is present
[+] 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 focuswriter-1.3.1-src.tar.bz2.*
    c245fbe9d8cf102c6648e1aac4d4b326  focuswriter-1.3.1-src.tar.bz2.packaged
    c245fbe9d8cf102c6648e1aac4d4b326  focuswriter-1.3.1-src.tar.bz2.upstream
[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
    koji scratch build:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2675584    
[.] MUST: If the package does not successfully compile, build or work ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] 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: library files with a suffix (e.g. libfoo.so.1.1), must go in a -devel
package.
[.] 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/-validate 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.

[.] SHOULD: If the source package does not include license text(s) as a
separate file ...
[+] 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.
[X] SHOULD: The reviewer should test that the package functions as described.
    I can crash focuswriter on my F13 64-bit KDE-spin machine by typing too 
    fast or just holding an arbitrary key.
[+] 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) 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.
[X] SHOULD: your package should contain man pages for binaries/scripts. 

----------------

Comments:

- As I mentioned in comment #3 the version number of qt-devel seems superfluous
to me since the EOL of F12.
- I have the feeling that the most common way is to put the %file section
_after_ %post, %postun, and %posttrans. This is, however, not required.
- A manpage would be nice to have.
- I will have a closer look on the stability issue, but since I found nobody
who could reproduce this behaviour this is not a blocker.

----------------
Package APPROVED
----------------

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.