Bug 979767 (kapow)
| Summary: | Review Request: kapow - A punch clock program | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Ankur Sinha (FranciscoD) <sanjay.ankur> |
| Component: | Package Review | Assignee: | Christopher Meng <i> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | i, notting, package-review |
| Target Milestone: | --- | Flags: | i:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | kapow-1.4.4.1-1.fc20 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2013-10-31 03:05:12 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
Ankur Sinha (FranciscoD)
2013-06-30 11:51:28 UTC
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. (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: > > 1. Source0 can be found: http://gottcode.org/kapow/kapow-1.4.4.1-src.tar.bz2 Updated > > 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. 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_usage 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 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?
URL 404. (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 [?]: 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}
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 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.
Forgot to say:
Use macro to ldflags:
export LDFLAGS="%{__global_ldflags}"
(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_by_a_package_which_is_not_required_for_your_package_to_function 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 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 New Package SCM Request ======================= Package Name: kapow Short Description: A punch clock program Owners: ankursinha Branches: f19 f20 InitialCC: 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 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 (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 Git done (by process-git-requests). 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 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 kapow-1.4.4.1-1.fc20 has been pushed to the Fedora 20 testing repository. kapow-1.4.4.1-1.fc19 has been pushed to the Fedora 19 stable repository. kapow-1.4.4.1-1.fc20 has been pushed to the Fedora 20 stable repository. |