https://bugzilla.redhat.com/show_bug.cgi?id=979767
Bug ID: 979767 Summary: Review Request: kapow - A punch clock program Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: sanjay.ankur@gmail.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://ankursinha.fedorapeople.org/kapow/kapow.spec SRPM URL: http://ankursinha.fedorapeople.org/kapow/kapow-1.4.4.1-1.fc19.src.rpm
Description: Kapow is a punch clock program designed to easily keep track of your hours, whether you're working on one project or many. Simply clock in and out with the Start/Stop button. If you make a mistake in your hours, you can go back and edit any of the entries by double-clicking on the session in question. Kapow also allows you to easily keep track of the hours since you last billed a client, by providing a helpful "Billed" check box--the totals will reflect your work after the last billed session.
Fedora Account System Username: ankursinha
rpmlint issues: [asinha@localhost SRPMS]$ rpmlint ../SPECS/kapow.spec ./kapow-1.4.4.1-1.fc19.src.rpm /var/lib/mock/fedora-19-x86_64/result/*.rpm ../SPECS/kapow.spec:49: W: macro-in-comment %find_lang ../SPECS/kapow.spec:50: W: macro-in-comment %find_lang ../SPECS/kapow.spec:66: W: macro-in-comment %files ../SPECS/kapow.spec:66: W: macro-in-comment %{name} ../SPECS/kapow.spec: W: invalid-url Source0: kapow-1.4.4.1-src.tar.bz2 kapow.src: W: spelling-error %description -l en_US checkbox -> check box, check-box, checkbook kapow.src:49: W: macro-in-comment %find_lang kapow.src:50: W: macro-in-comment %find_lang kapow.src:66: W: macro-in-comment %files kapow.src:66: W: macro-in-comment %{name} kapow.src: W: invalid-url Source0: kapow-1.4.4.1-src.tar.bz2 kapow.src: W: spelling-error %description -l en_US checkbox -> check box, check-box, checkbook kapow.src:49: W: macro-in-comment %find_lang kapow.src:50: W: macro-in-comment %find_lang kapow.src:66: W: macro-in-comment %files kapow.src:66: W: macro-in-comment %{name} kapow.src: W: invalid-url Source0: kapow-1.4.4.1-src.tar.bz2 kapow.x86_64: W: spelling-error %description -l en_US checkbox -> check box, check-box, checkbook kapow.x86_64: W: no-manual-page-for-binary kapow 4 packages and 1 specfiles checked; 0 errors, 19 warnings. [asinha@localhost SRPMS]$
Other rpms, for rawhide and F19 are available at http://ankursinha.fedorapeople.org/kapow/
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias| |kapow
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #1 from Christopher Meng cickumqt@gmail.com --- I'd like to say the author is good at making money.
However some problems:
1. Source0 can be found: http://gottcode.org/kapow/kapow-1.4.4.1-src.tar.bz2
2. Please leave less blank lines as far as possible.
3. No need to rm -rf $RPM_BUILD_ROOT
4. desktop-file-validate should be put into %check section.
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #2 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- (In reply to Christopher Meng from comment #1)
I'd like to say the author is good at making money.
It's perfectly acceptable for foss developers to request donations :)
However some problems:
- Source0 can be found: http://gottcode.org/kapow/kapow-1.4.4.1-src.tar.bz2
Updated
Please leave less blank lines as far as possible.
No need to rm -rf $RPM_BUILD_ROOT
desktop-file-validate should be put into %check section.
This is not a MUST. The guidelines say that desktop-file-validate can be used in either the install or check sections. I've moved it to a separate check section now any way.
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usag...
Spec/f19 srpm updated. Since they are only cosmetic changes, I haven't rebuilt the binary rpms.
I notice you haven't accepted the review ticket. Are you going to do a full review? :)
Thanks, Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |cickumqt@gmail.com Flags| |fedora-review?
--- Comment #3 from Christopher Meng cickumqt@gmail.com --- Yes. I'll do a full review.
And please fix the issues above.
Besides why there are
%{_datadir}/%{name}/translations/qt_it.qm %{_datadir}/%{name}/translations/qt_nl.qm
but not in lang file?
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #4 from Christopher Meng cickumqt@gmail.com --- URL 404.
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #5 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- (In reply to Christopher Meng from comment #3)
Yes. I'll do a full review.
And please fix the issues above.
Fixed.
Besides why there are
%{_datadir}/%{name}/translations/qt_it.qm %{_datadir}/%{name}/translations/qt_nl.qm
but not in lang file?
The don't get picked by find lang. I'm assuming they're something to do with qt5. I've commented in the spec.
I hadn't realized that I deleted the spec/srpm. Reuploaded them. Apologies:
Spec/srpm: http://ankursinha.fedorapeople.org/kapow/kapow.spec http://ankursinha.fedorapeople.org/kapow/kapow-1.4.4.1-1.fc21.src.rpm
Thanks, Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #6 from Christopher Meng cickumqt@gmail.com --- [?]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/icons/hicolor/256x256/apps, /usr/share/icons/hicolor/24x24, /usr/share/icons/hicolor/64x64/apps, /usr/share/icons/hicolor/22x22, /usr/share/icons/hicolor/48x48/apps, /usr/share/icons/hicolor/22x22/apps, /usr/share/icons/hicolor/32x32/apps, /usr/share/icons/hicolor/24x24/apps, /usr/share/kapow, /usr/share/kapow/translations, /usr/share/icons/hicolor/16x16/apps, /usr/share/icons/hicolor/scalable/apps, /usr/share/icons/hicolor/16x16, /usr/share/icons/hicolor/128x128/apps, /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/64x64, /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/256x256, /usr/share/icons/hicolor, /usr/share/icons/hicolor/32x32, /usr/share/icons/hicolor/scalable
AND
kapow.src:43: W: macro-in-comment %find_lang kapow.src:44: W: macro-in-comment %find_lang kapow.src:61: W: macro-in-comment %files kapow.src:61: W: macro-in-comment %{name}
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #7 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Spec updated:
http://ankursinha.fedorapeople.org/kapow/kapow.spec
http://ankursinha.fedorapeople.org/kapow/kapow-1.4.4.1-1.fc21.src.rpm
* Mon Oct 21 2013 Ankur Sinha <ankursinha AT fedoraproject DOT org> 1.4.4.1-1 - Update as per https://bugzilla.redhat.com/show_bug.cgi?id=979767#c6 - Remove comments - Own datadir/name directory - Own icon directories - Add an appdata file
[asinha@ankur-laptop SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/kapow.spec ./kapow-1.4.4.1-1.fc20.src.rpm kapow.x86_64: W: no-manual-page-for-binary kapow 4 packages and 1 specfiles checked; 0 errors, 1 warnings. [asinha@ankur-laptop SRPMS]$
Thanks, Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #8 from Christopher Meng cickumqt@gmail.com --- No, you are doing something wrong:
%dir %{_datadir}/icons/hicolor/ %dir %{_datadir}/icons/hicolor/16x16/ %dir %{_datadir}/icons/hicolor/16x16/apps/ %dir %{_datadir}/icons/hicolor/22x22/ %dir %{_datadir}/icons/hicolor/22x22/apps/ %dir %{_datadir}/icons/hicolor/24x24/ %dir %{_datadir}/icons/hicolor/24x24/apps/ %dir %{_datadir}/icons/hicolor/32x32/ %dir %{_datadir}/icons/hicolor/32x32/apps/ %dir %{_datadir}/icons/hicolor/48x48/ %dir %{_datadir}/icons/hicolor/48x48/apps/ %dir %{_datadir}/icons/hicolor/64x64/ %dir %{_datadir}/icons/hicolor/64x64/apps/ %dir %{_datadir}/icons/hicolor/128x128/ %dir %{_datadir}/icons/hicolor/128x128/apps/ %dir %{_datadir}/icons/hicolor/256x256/ %dir %{_datadir}/icons/hicolor/256x256/apps/ %dir %{_datadir}/icons/hicolor/scalable/ %dir %{_datadir}/icons/hicolor/scalable/apps/
These dirs are used by other pkgs, too:
[ ]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/icons/hicolor/256x256/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/24x24(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/64x64/apps(hicolor-icon-theme), /usr/share/icons/hicolor/22x22(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/48x48/apps(hicolor-icon-theme, fedora-logos, metromap), /usr/share/icons/hicolor/22x22/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/32x32/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/48x48(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/24x24/apps(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/16x16/apps(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/scalable/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/16x16(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/128x128/apps(hicolor-icon-theme), /usr/share/icons/hicolor/128x128(hicolor-icon-theme), /usr/share/icons/hicolor/64x64(hicolor-icon-theme), /usr/share/appdata (gnome-color-manager, ghex, baobab, gnome-contacts, gnome-calculator, gnome-documents, epiphany, gnome-system-monitor, simple-scan, gedit, filesystem, gnome-boxes), /usr/share/icons/hicolor/256x256(hicolor-icon- theme, fedora-logos), /usr/share/icons/hicolor(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/32x32(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/scalable(hicolor-icon-theme, fedora- logos)
My opinion is that you should keep original list but only add these two:
/usr/share/kapow, /usr/share/kapow/translations
to be owned.
Please fix issues above before import.
-----------
PACKAGE APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #9 from Christopher Meng cickumqt@gmail.com --- Forgot to say:
Use macro to ldflags:
export LDFLAGS="%{__global_ldflags}"
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #10 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- (In reply to Christopher Meng from comment #8)
No, you are doing something wrong:
%dir %{_datadir}/icons/hicolor/ %dir %{_datadir}/icons/hicolor/16x16/ %dir %{_datadir}/icons/hicolor/16x16/apps/ %dir %{_datadir}/icons/hicolor/22x22/ %dir %{_datadir}/icons/hicolor/22x22/apps/ %dir %{_datadir}/icons/hicolor/24x24/ %dir %{_datadir}/icons/hicolor/24x24/apps/ %dir %{_datadir}/icons/hicolor/32x32/ %dir %{_datadir}/icons/hicolor/32x32/apps/ %dir %{_datadir}/icons/hicolor/48x48/ %dir %{_datadir}/icons/hicolor/48x48/apps/ %dir %{_datadir}/icons/hicolor/64x64/ %dir %{_datadir}/icons/hicolor/64x64/apps/ %dir %{_datadir}/icons/hicolor/128x128/ %dir %{_datadir}/icons/hicolor/128x128/apps/ %dir %{_datadir}/icons/hicolor/256x256/ %dir %{_datadir}/icons/hicolor/256x256/apps/ %dir %{_datadir}/icons/hicolor/scalable/ %dir %{_datadir}/icons/hicolor/scalable/apps/
These dirs are used by other pkgs, too:
[ ]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/icons/hicolor/256x256/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/24x24(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/64x64/apps(hicolor-icon-theme), /usr/share/icons/hicolor/22x22(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/48x48/apps(hicolor-icon-theme, fedora-logos, metromap), /usr/share/icons/hicolor/22x22/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/32x32/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/48x48(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/24x24/apps(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/16x16/apps(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/scalable/apps(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/16x16(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/128x128/apps(hicolor-icon-theme), /usr/share/icons/hicolor/128x128(hicolor-icon-theme), /usr/share/icons/hicolor/64x64(hicolor-icon-theme), /usr/share/appdata (gnome-color-manager, ghex, baobab, gnome-contacts, gnome-calculator, gnome-documents, epiphany, gnome-system-monitor, simple-scan, gedit, filesystem, gnome-boxes), /usr/share/icons/hicolor/256x256(hicolor-icon- theme, fedora-logos), /usr/share/icons/hicolor(hicolor-icon-theme, fedora-logos), /usr/share/icons/hicolor/32x32(hicolor-icon-theme, fedora- logos), /usr/share/icons/hicolor/scalable(hicolor-icon-theme, fedora- logos)
My opinion is that you should keep original list but only add these two:
/usr/share/kapow, /usr/share/kapow/translations
to be owned.
I'm slightly confused about this. I checked up the guidelines: It's OK to co-own packages if the functionality of this package doesn't depend on the other packages owning the same directories. I haven't been able to figure out if kapow will pull in fedora-logos or hicolor in the dep chain. (This is also probably why both fedora-logos and hicolor own the directories, since they aren't dependent on each other)
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_b...
I double checked with my other specs, and yes, they don't own these directories. I'll remove them before import. I'll get this clarified for my own understanding later on though.
Please fix issues above before import.
PACKAGE APPROVED.
(In reply to Christopher Meng from comment #9)
Forgot to say:
Use macro to ldflags:
export LDFLAGS="%{__global_ldflags}"
Fixed.
Thank you for the review.
Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #11 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Updated spec/srpm
http://ankursinha.fedorapeople.org/kapow/kapow.spec
http://ankursinha.fedorapeople.org/kapow/kapow-1.4.4.1-1.fc21.src.rpm
* Tue Oct 22 2013 Ankur Sinha <ankursinha AT fedoraproject DOT org> 1.4.4.1-1 - Correct directory ownership - Correct ld flags - https://bugzilla.redhat.com/show_bug.cgi?id=979767#c8
Making the SCM request now.
Thanks again for the review, Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #12 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- New Package SCM Request ======================= Package Name: kapow Short Description: A punch clock program Owners: ankursinha Branches: f19 f20 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #13 from Michael Schwendt bugs.michael@gmx.net --- What Christopher fails to mention is that the "hicolor-icon-theme" is a "-filesystem" type of package. It only contains the basic structure for the hicolor icon theme tree, i.e. no icons but just directories, documentation, and a few support files.
That's covered in the yellow box here: https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_b...
As such, it is preferred if you "Requires: hicolor-icon-theme" to get right the ownership for those many dirs. The alternative would not be pretty:
%{_datadir}/icons/hicolor/*/apps/%{name}.*
Isn't sufficient. At least these would not be included:
%{_datadir}/icons/hicolor %{_datadir}/icons/hicolor/* %{_datadir}/icons/hicolor/*/apps
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #14 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- (In reply to Michael Schwendt from comment #13)
What Christopher fails to mention is that the "hicolor-icon-theme" is a "-filesystem" type of package. It only contains the basic structure for the hicolor icon theme tree, i.e. no icons but just directories, documentation, and a few support files.
That's covered in the yellow box here: https://fedoraproject.org/wiki/Packaging: Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your _package_to_function
I did read that part. I didn't realize hicolor-icon-theme was a filesystem type package.
As such, it is preferred if you "Requires: hicolor-icon-theme" to get right the ownership for those many dirs. The alternative would not be pretty:
I've added this to the spec.
%{_datadir}/icons/hicolor/*/apps/%{name}.*
Isn't sufficient. At least these would not be included:
%{_datadir}/icons/hicolor %{_datadir}/icons/hicolor/* %{_datadir}/icons/hicolor/*/apps
Thanks for the clarification Michael.
Warm regards, Ankur
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #15 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- kapow-1.4.4.1-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/kapow-1.4.4.1-1.fc19
https://bugzilla.redhat.com/show_bug.cgi?id=979767
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- kapow-1.4.4.1-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/kapow-1.4.4.1-1.fc20
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- kapow-1.4.4.1-1.fc20 has been pushed to the Fedora 20 testing repository.
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |kapow-1.4.4.1-1.fc19 Resolution|--- |ERRATA Last Closed| |2013-10-30 23:05:12
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- kapow-1.4.4.1-1.fc19 has been pushed to the Fedora 19 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=979767
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|kapow-1.4.4.1-1.fc19 |kapow-1.4.4.1-1.fc20
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- kapow-1.4.4.1-1.fc20 has been pushed to the Fedora 20 stable repository.
package-review@lists.fedoraproject.org