Bug 613525 - Review Request: klog - KLog is a Ham radio logging program for KDE
Summary: Review Request: klog - KLog is a Ham radio logging program for KDE
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: manuel wolfshant
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-07-12 07:09 UTC by Randy Berry
Modified: 2010-07-20 22:50 UTC (History)
2 users (show)

Fixed In Version: klog-0.5.6-4.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-07-20 22:50:30 UTC
Type: ---
Embargoed:
manuel.wolfshant: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Randy Berry 2010-07-12 07:09:59 UTC
Spec URL: http://dp67.fedorapeople.org/pkgs/SPECS/klog.spec
SRPM URL: http://dp67.fedorapeople.org/pkgs/SRPMS/klog-0.5.6-1.fc13.src.rpm

Description: 
KLog is a Ham radio logging program for KDE
Some features include:
	* DXCC award support.
	* Basic IOTA support.
	* Importing from Cabrillo files.
	* Importing from TLF.
	* Adding/Editing QSOs.
	* Save/read to/from disk file the log - ADIF format by default.
	* English/Spanish/Portuguese/Galician/Serbian/Swedish support.
	* QSL sent/received support.
	* Read/Write ADIF.
	* Delete QSOs.
	* DX-Cluster support. 
Some additional features of this application are still under development
and are not yet implemented.

klog.i686: W: no-manual-page-for-binary klog
3 packages and 1 specfiles checked; 0 errors, 1 warnings.

Manual page is under construction.

Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=2312384
Project Website: http://jaime.robles.es/klog.php
Source Download: http://jaime.robles.es/klog/download/klog-0.5.6.tar.gz
Pkg list request: http://lists.fedoraproject.org/pipermail/packaging/2010-June/007200.html

Comment 1 Randy Berry 2010-07-12 08:43:53 UTC
* Mon Jul 12 2010 Randall J. Berry, N3LRX <dp67 at fedoraproject dot org>  - 0.5.6-2
- Forgot to apply locale files.

Spec: http://dp67.fedorapeople.org/pkgs/SPECS/klog.spec
SRPM: http://dp67.fedorapeople.org/pkgs/SRPMS/klog-0.5.6-2.fc13.src.rpm

Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=2312520

Comment 2 manuel wolfshant 2010-07-16 15:44:32 UTC
Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: devel/x86_64
 [x] Rpmlint output:
source RPM:
klog.src: W: spelling-error %description -l en_US Cabrillo -> Carrillo, Cabriolet, Carillon
klog.src: W: spelling-error %description -l en_US Galician -> Galilean, Gallicism, Alicia
binary RPM:
klog.x86_64: W: spelling-error %description -l en_US Cabrillo -> Carrillo, Cabriolet, Carillon
klog.x86_64: W: spelling-error %description -l en_US Galician -> Galilean, Gallicism, Alicia
=> all ignorable
 [x] Package is not relocatable.
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type:GPLv2+
 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package match the upstream source, as provided in the spec URL.
     SHA1SUM of source file: dfee05ec729c9a92408ade41618728adb4adb193  klog-0.5.6.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
 [x] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [!] Package must own all directories that it creates.
=> See issue 1
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [!] Package contains a properly installed %{name}.desktop file if it is a GUI application.
=> See issue 2
 [x] Package does not own files or directories owned by other packages.
 [x] Final provides and requires are sane.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [x] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: koji scratch build
 [x] Package should compile and build into binary rpms on all supported architectures.
     Tested on: koji scratch build
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [-] %check is present and the test passes.

=== OPTIONAL ITEMS ===
 [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package has a %clean section, which contains rm -rf or $RPM_BUILD_ROOT.

=== Issues ===
1. The following files are included
-rw-r--r--    1 root    root                     5275 Jul  4 21:47 /etc/skel/.klog/awa/tpea.awa
-rw-r--r--    1 root    root                     5368 Jul  4 21:47 /etc/skel/.klog/awa/was.awa
-rw-r--r--    1 root    root                    54267 Jul  4 21:47 /etc/skel/.klog/data/cty.dat
-rw-r--r--    1 root    root                      324 Jul  4 21:47 /etc/skel/.klog/data/klog-contest-cabrillo-formats.txt
but the directories /etc/skel/.klog and below are not owned.
2. desktop-file-install should be used to install (and verify) the desktop file or at least desktop-file-validate should be used to validate the one installed by the application's installer


=== Final Notes ===
 Please fix the issues noted above and we are good to go.

Comment 3 Randy Berry 2010-07-16 19:20:27 UTC
Thanks Wolfy,

Spec: http://dp67.fedorapeople.org/pkgs/SPECS/klog.spec
SRPM: http://dp67.fedorapeople.org/pkgs/SRPMS/klog-0.5.6-3.fc13.src.rpm

rpmlint:

klog.i686: W: hidden-file-or-dir /etc/skel/.klog
klog.i686: W: no-manual-page-for-binary klog
3 packages and 1 specfiles checked; 0 errors, 2 warnings.

Hidden directory is intentional to conform with /etc/skel/ structure.

Not sure why I'm not getting the spelling warnings with rpmlint but they are intentional and should be ignored.

Upstream is working on adding a man page.

Comment 4 manuel wolfshant 2010-07-17 13:03:55 UTC
+#               klog.sh.in  -  Install wraapper script for klog
I bet "wraapper" is not in the dictionary, even in HAM operators' one :)

In the following sequence
  mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/
  mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/data/
  mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/awa/
there is no actual error, but 
a) the first command is not needed, given that the second and third ones would create the whole directory structure, due to the "-p" parameter. 
b) OTOH, if you preserve the first command in the sequence, the second and third one no longer need "-p" because the whole directory structure would have been created by the first command.

The following part
  desktop-file-validate \
	$RPM_BUILD_ROOT/%{_datadir}/applications/kde4/%{name}.desktop
is superflous, given that you have used desktop-file-install to install AND verfy the file. Please either use desktop-file-install to verify the file and put it in place OR desktop-file-validate if the installer already copies the file to the correct place.
As a matter of style I think that you should have used either use %{name} or "klog" in both desktop-file-* invocations.

Comment 5 Randy Berry 2010-07-17 21:51:58 UTC
Spec: http://dp67.fedorapeople.org/pkgs/SPECS/klog.spec
SRPM: http://dp67.fedorapeople.org/pkgs/SRPMS/klog-0.5.6-4.fc13.src.rpm

(In reply to comment #4)
> +#               klog.sh.in  -  Install wraapper script for klog
> I bet "wraapper" is not in the dictionary, even in HAM operators' one :)

 Done: Fixed my twitchy finger action. :)

> In the following sequence
>   mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/
>   mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/data/
>   mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir}/skel/.%{name}/awa/
> there is no actual error, but 
> a) the first command is not needed, given that the second and third ones would
> create the whole directory structure, due to the "-p" parameter. 
> b) OTOH, if you preserve the first command in the sequence, the second and
> third one no longer need "-p" because the whole directory structure would have
> been created by the first command.

 Done: Removed first command.


> The following part
>   desktop-file-validate \
>  $RPM_BUILD_ROOT/%{_datadir}/applications/kde4/%{name}.desktop
> is superflous, given that you have used desktop-file-install to install AND
> verfy the file. Please either use desktop-file-install to verify the file and
> put it in place OR desktop-file-validate if the installer already copies the
> file to the correct place.

 Done: Used desktop-file-install instead of both install/validate

> As a matter of style I think that you should have used either use %{name} or
> "klog" in both desktop-file-* invocations.

 Done: Changed klog in desktop-file-install to %{name}

Comment 6 manuel wolfshant 2010-07-19 07:01:23 UTC
APPROVED

Comment 7 Randy Berry 2010-07-19 22:00:39 UTC
Thanks Wolfy,

New Package CVS Request
=======================
Package Name: klog
Short Description: KLog is a Ham radio logging program for KDE
Owners: dp67
Branches: F-12 F-13

Comment 8 Kevin Fenzi 2010-07-20 03:19:47 UTC
CVS done (by process-cvs-requests.py).

Comment 9 Fedora Update System 2010-07-20 05:13:51 UTC
klog-0.5.6-4.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/klog-0.5.6-4.fc13

Comment 10 Fedora Update System 2010-07-20 22:50:20 UTC
klog-0.5.6-4.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.