Bug 528150
Summary: | Review Request: invulgotracker - Tasks & projects tracking tool | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Tareq Al Jurf <taljurf> |
Component: | Package Review | Assignee: | Christoph Wickert <christoph.wickert> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | 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
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. 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 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. 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. 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. 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. 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. Package Updated: 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 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. Fixed: SRPM http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker-0.53.1-3.fc13.src.rpm SPEC: http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker.spec 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. Fixed and updated to 0.60 SRPM http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker-0.60-1.fc13.src.rpm SPEC http://taljurf.fedorapeople.org/Packages/i586/invulgotracker/invulgotracker.spec 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. fixed and updated SRPM http://taljurf.fedorapeople.org/Packages/i686/invulgotracker/invulgotracker-0.61-1.fc13.src.rpm SPEC http://taljurf.fedorapeople.org/Packages/i686/invulgotracker/invulgotracker.spec 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. New Package CVS Request ======================= Package Name: invulgotracker Short Description: Tasks & projects tracking tool Owners: taljurf Branches: F-11 F-12 InitialCC: taljurf 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. cvs done. invulgotracker-0.61-1.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/invulgotracker-0.61-1.fc12 invulgotracker-0.61-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/invulgotracker-0.61-1.fc11 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. 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. |