Bug 627637

FE-NEEDSPONSOR

An unofficial review:

+ OK
? ISSUE
- NA

+ 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
? License file included in package
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:
[Ankur@070905042 rpmbuild]$ md5sum qroneko-0.5.4.tar.gz SOURCES/qroneko-0.5.4.tar.gz 
5e2e20d9ce40e076cbe79d64a21f6e6b  qroneko-0.5.4.tar.gz
52ef339ece38cb408ab17afc81165edb  SOURCES/qroneko-0.5.4.tar.gz


- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
? Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

+ Package is a GUI app and has a .desktop file

- Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
- No rpmlint output.

SHOULD Items:

+ Should build in mock.
- Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1. You've gotten the license wrong. The source files specify GPLv2+ as the license. Please confirm. Also consider including a copy of the license in the package. You should request upstream to consider including a copy in the tar next release too. 

2.You've missed out the docs in the package. The copy of the license and the README should be included as docs.

3. Description/Summary could be changed to "A qt front end to crontab" or something similiar, more informative than what it is currently. 

4. URL needs to be corrected as per https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

5. You forgot to add your name to the changelog ;)

6. rpmlint output :


[Ankur@070905042 SRPMS]$ rpmlint qroneko-0.5.4-1.fc13.src.rpm ../SPECS/qroneko.spec /var/lib/mock/fedora-rawhide-i386/result/*.rpm
qroneko.src: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, cronyism
qroneko.src: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, cronyism
qroneko.i686: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, cronyism
qroneko.i686: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, cronyism
qroneko.i686: W: no-documentation
qroneko.i686: E: non-executable-script /usr/share/applications/qroneko.desktop 0644L /usr/bin/env
qroneko.i686: W: no-manual-page-for-binary qroneko
qroneko.src: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, cronyism
qroneko.src: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, cronyism
4 packages and 1 specfiles checked; 1 errors, 8 warnings.


Please look at these and correct the errors.

I fixed issues which you pointed out and here is updated spec and srpm 

Spec URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko.spec
SRPM URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko-0.5.4-2.fc13.src.rpm

1. rpmlint output:


[daredevil@localhost SPECS]$ rpmlint qroneko.spec ../RPMS/i686/qroneko-0.5.4-2.fc13.i686.rpm ../SRPMS/qroneko-0.5.4-2.fc13.src.rpm
qroneko.i686: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, Cronus
qroneko.i686: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, Cronus
qroneko.i686: W: no-manual-page-for-binary qroneko
qroneko.src: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, Cronus
qroneko.src: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, Cronus
2 packages and 1 specfiles checked; 0 errors, 5 warnings.

Hi Praveen,

the tarball in the SRPM doesn't match the upstream one:
$ md5sum qroneko-0.5.4.tar.gz*
e223113fb16e96d6780da3d29eb4c214  qroneko-0.5.4.tar.gz
5e2e20d9ce40e076cbe79d64a21f6e6b  qroneko-0.5.4.tar.gz.1

You must use the unchanged upstream tarball to build you package. Also, don't add a file containing the license text if it's not present in the tarball. However, you should ask the upstream developer to add it to a future release.

If you need to change some source files, add patches to your package.

Here are some more comments:
- Drop BR: qt as it's automatically added as a dependency of qt-devel.

- I suggest to use the %description text from the upstream website:
  qroneko is a scheduling utility which uses crontab as the backend.
  
  Features and highlights:
  * Smart "Cron Time" Setting.
  * Lists expected execute time.
  * Enables to edit crontab by text

- In the %prep section, it's sufficient to use %setup -q

- Don't add full paths to install and qmake_qt4 since they may be different. 
  Just use "install" and %{_qt4_qmake}

- The tarball doesn't contain a .desktop file. Thus, you have to add one to 
  your package. See https://fedoraproject.org/wiki/Packaging/Guidelines#desktop
  (permissions of the .desktop file should be 0644)

- add a blank line between the %changelog entries to increase legibility

- The package fails building for x86_64. The Compilation aborts with error 
  messages like this:
  CronModel.cpp:248: error: cast from 'TCommand*' to 'int' loses precision

  This should be reported and fixed upstream. However, the upstream
  developer submitted the latest changes 4 years ago, so the project probably 
  isn't active any longer.

Fixed all the issue which Mr. Martin Gieseking pointed out, here is updated spec file and srpm

Spec URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko.spec
SRPM URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko-0.5.4-3.fc13.src.rpm
Source1 URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko.desktop
Patch0 URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko-0.5.4-CronModel.cpp.patch
Patch1 URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko-0.5.4-ExecuteList.cpp.patch

1.rpmlint output:

[daredevil@localhost SPECS]$ rpmlint ../SRPMS/qroneko-0.5.4-3.fc13.src.rpm 
qroneko.src: W: spelling-error Summary(en_US) crontab -> Cronkite, frontal, Cronus
qroneko.src: W: spelling-error %description -l en_US crontab -> Cronkite, frontal, Cronus
1 packages and 0 specfiles checked; 0 errors, 2 warnings.


2.md5sum output:

[daredevil@localhost SPECS]$ md5sum ../SOURCES/qroneko-0.5.4.tar.gz ../../Downloads/qroneko-0.5.4.tar.gz 
5e2e20d9ce40e076cbe79d64a21f6e6b  ../SOURCES/qroneko-0.5.4.tar.gz
5e2e20d9ce40e076cbe79d64a21f6e6b  ../../Downloads/qroneko-0.5.4.tar.gz

Hi Praveen,

Please go through the following link 
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

It notes the requirements to get sponsored to the packager group.

regards,
Ankur

Some notes:

* Clean up of old miscs
  - Please clean up old miscs no longer needed on Fedora, like
    * BuildRoot tag
    * "rm -rf $RPM_BUILD_ROOT" at the top of %install
    * %clean

* $RPM_BUILD_ROOT vs %buildroot / coding style
  - Please choose to use one style, not both.
        https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
  - Also, using both "install -d" and "mkdir" seems a bit
    confusing, I recommend to use one style.

* Timestamp
  - When using "cp" or "install" commands, please add "-p" option
    to keep timestamp on installed files.
    https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

* Path
  - Would you explain why you want to install "qroneko" binary to %_sbindir,
    not under %_bindir? (it seems that qroneko can be lauched as normal user).

Additional notes:

* Desktop file
  - "#!/usr/bin/env xdg-open" is not needed.
  - "Application" in Categories item is deprecated and should be removed.

Fixed all the issue which Mr. Mamoru Tasaka pointed out, here is updated
spec file and srpm

SPEC URL: http://kumarpraveen.fedorapeople.org/qroneko/qroneko.spec
SRPM URL:
http://kumarpraveen.fedorapeople.org/qroneko/qroneko-0.5.4-4.fc14.src.rpm

1.rpmlint output:

$ rpmlint qroneko.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.


2.md5sum output:

$ md5sum ../SOURCES/qroneko-0.5.4.tar.gz
../../Downloads/qroneko-0.5.4.tar.gz 
5e2e20d9ce40e076cbe79d64a21f6e6b  ../SOURCES/qroneko-0.5.4.tar.gz
5e2e20d9ce40e076cbe79d64a21f6e6b  ../../Downloads/qroneko-0.5.4.tar.gz

Well,
- this package is now okay
  ! Note that "-p" option is not needed for "install" used with
    "-d" (i.e. when creating directory) (needed when installing
    file), however not a blocker.
- you have another review request submission, now in progress.

----------------------------------------------------------
    This package (qroneko) is APPROVED by mtasaka
----------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 13/14, you also have
to look at
http://fedoraproject.org/wiki/Bodhi_Guide
(after once you rebuilt this package on koji Fedora rebuilding system).

When using Fedora SCM system, please check below for reference:
http://fedoraproject.org/wiki/Using_Fedora_GIT

If you have questions, please ask me.

Removing NEEDSPONSOR.

New Package SCM Request
=======================
Package Name: qroneko
Short Description: A front end of crontab application
Owners: kumarpraveen
Branches: f13 f14

Ah, please also request for f15 branch.

New Package SCM Request
=======================
Package Name: qroneko
Short Description: A front end of crontab application
Owners: kumarpraveen
Branches: f13 f14 f15

Git done (by process-git-requests).

qroneko-0.5.4-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/qroneko-0.5.4-4.fc14

qroneko-0.5.4-4.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/qroneko-0.5.4-4.fc13

qroneko-0.5.4-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/qroneko-0.5.4-4.fc15

Closing.

qroneko-0.5.4-4.fc14 has been pushed to the Fedora 14 stable repository.

qroneko-0.5.4-4.fc15 has been pushed to the Fedora 15 stable repository.

qroneko-0.5.4-4.fc13 has been pushed to the Fedora 13 stable repository.