Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Summary: Review Request: TrustedQSL - TrustedQSL ham-radio applications Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: cooly@gnome.eu.org QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://lucilanga.fedorapeople.org/TrustedQSL.spec SRPM URL: http://lucilanga.fedorapeople.org/TrustedQSL-1.11-1.fc9.src.rpm Description: The TrustedQSL applications are used for generating digitally signed QSO records (records of Amateur Radio contacts). This package contains the GUI applications tqslcert and tqsl.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: TrustedQSL - TrustedQSL ham-radio applications
https://bugzilla.redhat.com/show_bug.cgi?id=455396
cooly@gnome.eu.org changed:
What |Removed |Added ---------------------------------------------------------------------------- BugsThisDependsOn| |455380
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Bug 455396 depends on bug 455380, which changed state.
Bug 455380 Summary: Review Request: tqsllib - The TrustedQSL library https://bugzilla.redhat.com/show_bug.cgi?id=455380
What |Old Value |New Value ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Resolution| |NEXTRELEASE Status|ASSIGNED |CLOSED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Jon Ciesla limb@jcomserv.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |limb@jcomserv.net AssignedTo|nobody@fedoraproject.org |limb@jcomserv.net Flag| |fedora-review?
--- Comment #1 from Jon Ciesla limb@jcomserv.net 2008-10-10 11:35:57 EDT --- Build error:
+ mkdir -p /var/tmp/TrustedQSL-1.11-1.fc9-root-limb/usr/share/applications + mkdir -p /var/tmp/TrustedQSL-1.11-1.fc9-root-limb/usr/share/pixmaps + cp icons/key48.png /var/tmp/TrustedQSL-1.11-1.fc9-root-limb/usr/share/pixmaps/TrustedQSL.png + sed -i -e s/.png//g tqsl.desktop + sed -i -e s/.png//g tqslcert.desktop + desktop-file-install --vendor=fedora --dir=/var/tmp/TrustedQSL-1.11-1.fc9-root-limb/usr/share/applications /home/limb/rpmbuild/BUILD/tqsl.desktop Error on file "/home/limb/rpmbuild/BUILD/tqsl.desktop": No such file or directory error: Bad exit status from /var/tmp/rpm-tmp.32062 (%install)
If you drop the %{builddir} it works.
Also, you need to BuildRequire desktop-file-utils and expat-devel.
rpmlint is clean on everything.
Since upstream uses lowercase some places in their naming and uppercase in others, from what I see, it might be better to make this package's name all lowercase. Your call.
Comment on status of your patches WRT upstream. If submitted upstream, indicate this in the spec. If not, do so, and indicate in the spec.
Add ChangeLog, INSTALL and the help/ dir to %doc.
Otherwise, full review looks good. I'm doing another mock build, I'll post back with any further BR issues.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #2 from Jon Ciesla limb@jcomserv.net 2008-10-10 11:57:52 EDT --- No more BR issues.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #3 from Lucian Langa cooly@gnome.eu.org 2008-10-10 15:42:19 EDT --- (In reply to comment #1)
If you drop the %{builddir} it works.
fixed
Also, you need to BuildRequire desktop-file-utils and expat-devel.
fixed
Since upstream uses lowercase some places in their naming and uppercase in others, from what I see, it might be better to make this package's name all lowercase. Your call.
lowered the name
Comment on status of your patches WRT upstream. If submitted upstream, indicate this in the spec. If not, do so, and indicate in the spec.
fixed
Add ChangeLog, INSTALL and the help/ dir to %doc.
Added ChangeLog and help/*, INSTALL is only required for people that want to compile this from source.
also add detection for zlib, expat on x86_64 arch (koji failed to build those)
.. and bumped version to:
http://lucilanga.fedorapeople.org/trustedqsl.spec http://lucilanga.fedorapeople.org/trustedqsl-1.11-2.fc10.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Jon Ciesla limb@jcomserv.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #4 from Jon Ciesla limb@jcomserv.net 2008-10-10 16:04:51 EDT --- Drop an 'o' from losing, line 11.
Otherwise, looks great.
APPROVED.
Thanks!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Lucian Langa cooly@gnome.eu.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #5 from Lucian Langa cooly@gnome.eu.org 2008-10-11 02:59:21 EDT --- New Package CVS Request ======================= Package Name: trustedqsl Short Description: TrustedQSL ham-radio applications Owners: lucilanga Branches: F-8 F-9 EL-5 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #6 from Kevin Fenzi kevin@tummy.com 2008-10-12 21:55:25 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #7 from Fedora Update System updates@fedoraproject.org 2008-10-13 01:27:12 EDT --- trustedqsl-1.11-2.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/trustedqsl-1.11-2.fc8
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #8 from Fedora Update System updates@fedoraproject.org 2008-10-13 01:28:02 EDT --- trustedqsl-1.11-2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/trustedqsl-1.11-2.fc9
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
Lucian Langa cooly@gnome.eu.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #9 from Fedora Update System updates@fedoraproject.org 2008-11-05 23:03:37 EDT --- trustedqsl-1.11-2.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=455396
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2008-11-05 23:09:04 EDT --- trustedqsl-1.11-2.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org