Bug 528150

Summary: Review Request: invulgotracker - Tasks & projects tracking tool
Product: [Fedora] Fedora Reporter: Tareq Al Jurf <taljurf>
Component: Package ReviewAssignee: Christoph Wickert <christoph.wickert>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: christoph.wickert, fedora-package-review, jan.klepek, martin.gieseking, notting, pahan
Target Milestone: ---Flags: christoph.wickert: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.61-1.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-01-19 19:32:26 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 Tareq Al Jurf 2009-10-09 11:20:49 UTC
Spec URL: http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker.spec
SRPM URL: http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker-0.53-1.fc11.src.rpm
Description: 

Hi, I made this package and i hope to add it to the upcoming Fedora 12 repository.

Simple application to track time usage on your tasks & projects.

Comment 1 Martin Gieseking 2009-10-10 09:31:24 UTC
Some initial comments:

- according to the source file headers, the license is GPLv3+

- The files src/util.* are licensed under GPLv2+. You should ask upstream if this is intended.

- in Source0, give a full URL to the source tarball

- remove the final dot in Summary

- append CFLAGS="$RPM_OPT_FLAGS" CPPFLAGS="$RPM_OPT_FLAGS" to %configure in order to get the debuginfo package build correctly

- use macros in the %files section and add the doc files properly

- drop INSTALL and the empty files from %doc

- drop the locale file from %files

- see the packaging guidelines how to install the .desktop file  (http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files)

$ rpmlint /var/lib/mock/fedora-11-x86_64/result/invulgotracker-*
invulgotracker.src: W: summary-ended-with-dot Tasks & projects tracking tool.
invulgotracker.x86_64: W: summary-ended-with-dot Tasks & projects tracking tool.
invulgotracker.x86_64: E: zero-length /usr/doc/invulgotracker/NEWS
invulgotracker.x86_64: E: zero-length /usr/doc/invulgotracker/README
invulgotracker.x86_64: W: non-standard-dir-in-usr doc
invulgotracker-debuginfo.x86_64: E: debuginfo-without-sources
3 packages and 0 specfiles checked; 3 errors, 3 warnings.

Comment 2 Jan Klepek 2009-10-12 10:00:24 UTC
I can't find your FAS username in Fedora Packager CVS Commit Group (packager), did you joined this group? Who is your sponsor?

If this is your first package, I really suggest you read
http://fedoraproject.org/wiki/Join_the_package_collection_maintainers, follow
the instructions there.

I have set FE-NEEDSPONSOR, if you are member of Fedora Packager CVS Commit Group (packager), please provide your username in that group

Comment 3 Christoph Wickert 2009-10-14 15:34:29 UTC
Tarey is not yet in the packager group, but I'm going to sponsor him. He still needs to learn a lot, but he already did very well during the last two weeks, so I'm confident of him being a good packager soon.

Comment 4 Tareq Al Jurf 2009-11-23 07:58:08 UTC
Spec URL:
http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker.spec
SRPM URL:
http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker-0.53.1-1.fc13.src.rpm

Description: 

These are the final ones. For F13.
And I've applied to Fedora Packager CVS Commit Group.

Simple application to track time usage on your tasks & projects.

Comment 5 Christoph Wickert 2009-11-24 10:41:09 UTC
REVIEW FOR 
394a8a56bfaf7df638640d9ebc4b8c15  invulgotracker-0.53.1-1.fc13.src.rpm


OK - MUST: rpmlint is silent:
$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/invulgotracker-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines (GPLv3+)
OK - MUST: License field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: sources match the upstream source by MD5 
79ac959f374ee8ab931287e35131d1d3
OK - MUST: successfully compiles and builds into binary rpms on x86_64
N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
OK - MUST: all build dependencies are listed in BuildRequires.
OK - MUST: handles locales properly with %find_lang
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
OK - MUST: consistently uses macros
OK - MUST: package contains code
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package.
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
OK - MUST: The package contains a GUI application and includes a %{name}.desktop file, and that file is properly validated with desktop-file-validate in the %install section.
OK - MUST: package does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock.
OK - SHOULD: compiles and builds into binary rpms on all supported architectures.
OK - SHOULD: functions as described.
N/A - SHOULD: Scriptlets are used, those scriptlets must be sane.
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
N/A - 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.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - Compiler flags ok
OK - Debuginfo complete


Issues:
Timestamp of Source0 does not match timestamp of SourceURL. Please download the source again with wget or spectool -g invulgotracker.spec
For more info, see 
https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

/usr/share/icons/InvulgoTracker.png should be
/usr/share/pixmaps/InvulgoTracker.png
or
/usr/share/icons/hicolor/32x32/apps/InvulgoTracker.png
There should be no icons in the toplevel folder /usr/share/icons. Please patch Makefile to do this.

InvulgoTracker.desktop is very sparse. Please add "ProjectManagement" as additional category to allow nested menus. You can do this if you use desktop-file-install instead of desktop-file-validate as decribed in 
https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage

BTW: The icon tag in the desktop file is "InvulgoTracker.png", but should be InvulgoTracker. This is is why you see a warning from desktop-file-validate during build:
<----
desktop-file-validate /builddir/build/BUILDROOT/invulgotracker-0.53.1-1.fc13.i386/usr/share/applications/InvulgoTracker.desktop
/builddir/build/BUILDROOT/invulgotracker-0.53.1-1.fc13.i386/usr/share/applications/InvulgoTracker.desktop: warning: value "InvulgoTracker.png" for key "Icon" in group "Desktop Entry" is an icon name with an extension, but there should be no extension as described in the Icon Theme Specification if the value is not an absolute path
---->
For the background see 
https://fedoraproject.org/wiki/Packaging/Guidelines#Icon_tag_in_Desktop_Files
and for a fix
https://fedoraproject.org/wiki/PackageMaintainers/PackagingTricks#.desktop_files


From a formal point of view your package is already good as is, but please fix the remaining issues.

Comment 6 Christoph Wickert 2009-11-24 10:45:49 UTC
One last thing: If you do
rm -rf $RPM_BUILD_ROOT/%{_docdir}
you can drop
 --docdir=%{_docdir}/%{name}-%{version}
from configure. It's up to you to ether use 
 --docdir=%{_docdir}/%{name}-%{version} and %exclude the files you don't want or use %doc to install them. But please don't mix both to keep the spec simple.

Comment 7 Christoph Wickert 2009-11-24 10:49:46 UTC
One more thing. :)
You have deleted the first %changelog entry. Please don't do this, 0.53.1-1 is not the "initial build", 0.53-1 was. In the future, make sure to list all changes in the changelog.

Comment 9 Christoph Wickert 2009-11-29 11:51:52 UTC
Your versioning is wrong: The package should now be 0.53.1-2

First version was 0.53-1. Then upsteam made a new release to fix the docdir problem. This was 0.51.1, so your second package was 0.53.1-1. Then you fixed some issues in 0.53.1-2 and now you fixed the icon thing, so you should be at 0.35.1-3. Please change this before I can approve the package.

Comment 11 Christoph Wickert 2009-12-01 00:43:55 UTC
The patch is not yet working:

-    strcat(filename, "/icons/InvulgoTracker.png");
+    strcat(filename, "/icons/InvulgoTracker.");

This needs to be "/pixmaps/InvulgoTracker.png" both times. I guess you have confused this with the desktop file.

Comment 13 Christoph Wickert 2009-12-17 21:52:38 UTC
The package itself looks fine and fixes all problems, but there is a trival rpmlint warning that you should fix:

$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/invulgotracker-*
invulgotracker.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 10)
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

Make sure to use spaces in line 10 too. And the changelog format is not quite right: Instead of 
*Thu Dec 03 2009
it should be
* Thu Dec 03 2009

Note the whitespace before the date. You can fix this when updating to 0.61 which was just released.

Comment 15 Christoph Wickert 2009-12-18 10:21:24 UTC
The rpmlint warning still remains:
$ rpmlint Desktop/invulgotracker-0.61-1.fc13.src.rpm 
invulgotracker.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 10)
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

But this is trivial, you can change it later. The package is APPROVED.

Comment 16 Tareq Al Jurf 2009-12-23 20:15:28 UTC
New Package CVS Request
=======================
Package Name: invulgotracker
Short Description: Tasks & projects tracking tool
Owners: taljurf
Branches: F-11 F-12
InitialCC: taljurf

Comment 17 Christoph Wickert 2009-12-23 22:29:42 UTC
You forgot to set the Flag (somewhere at the top right of this page) fedora‑cvs to ?, otherwise the CVS admin don't see your request.

Comment 18 Kevin Fenzi 2009-12-24 07:04:49 UTC
cvs done.

Comment 19 Fedora Update System 2010-01-17 13:18:37 UTC
invulgotracker-0.61-1.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/invulgotracker-0.61-1.fc12

Comment 20 Fedora Update System 2010-01-17 13:18:48 UTC
invulgotracker-0.61-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/invulgotracker-0.61-1.fc11

Comment 21 Fedora Update System 2010-01-19 19:32:20 UTC
invulgotracker-0.61-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2010-01-19 19:43:44 UTC
invulgotracker-0.61-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.