Bug 627637
Summary: | Review Request: qroneko - A front end of crontab application | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Praveen Kumar <kumarpraveen.nitdgp> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, martin.gieseking, notting, pahan, sanjay.ankur, shreyankg |
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
j: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | qroneko-0.5.4-4.fc13 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-02-25 09:51:53 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
Praveen Kumar
2010-08-26 15:06:16 UTC
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. |