Bug 454250 - Review Request: tangoGPS - tangoGPS is a lightweight and fast mapping application
Summary: Review Request: tangoGPS - tangoGPS is a lightweight and fast mapping applica...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Debarshi Ray
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-07-07 08:46 UTC by Peter Robinson
Modified: 2008-09-01 23:20 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-09-01 23:20:06 UTC
Type: ---
Embargoed:
debarshir: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
Mock build log (21.25 KB, application/octet-stream)
2008-08-12 19:53 UTC, kushaldas@gmail.com
no flags Details
tangogps.desktop fix (550 bytes, patch)
2008-09-01 04:19 UTC, Debarshi Ray
no flags Details | Diff

Description Peter Robinson 2008-07-07 08:46:51 UTC
Spec URL: http://pbrobinson.fedorapeople.org/tangogps.spec
SRPM URL: http://pbrobinson.fedorapeople.org/tangogps-0.9.2-1.fc9.src.rpm

tangoGPS is a lightweight and fast mapping application. It runs on handheld
devices like the Openmoko Neo1973 as well as on the eeePC and the Linux
desktop. By default is uses map data from the OpenStreetMap.org project;
additionally a variety of other repositories can easily be added.

Comment 1 kushaldas@gmail.com 2008-08-12 19:53:26 UTC
Created attachment 314134 [details]
Mock build log

Comment 2 kushaldas@gmail.com 2008-08-12 19:54:31 UTC
rpmlint on srpm gives 
tangogps.src: W: summary-not-capitalized tangoGPS is a lightweight and fast mapping application
Also see the above attached mock build log file

Comment 3 Peter Robinson 2008-08-12 20:12:25 UTC
For the capitalised description the name is spelt tangoGPS so that's why its not capitalised. I now have a mock env setup so I'm putting it through the paces in my mock now and will update the spec/src with the fixes. Are you going to do the formal review for the package?

Comment 4 Peter Robinson 2008-08-12 20:20:47 UTC
Problem build requires fixed. Builds in mock on x86_64 for me. spec/src is the same as originally specified.

Comment 5 kushaldas@gmail.com 2008-08-13 06:04:11 UTC
(In reply to comment #4)
> Problem build requires fixed. Builds in mock on x86_64 for me. spec/src is the
> same as originally specified.

You are supposed to bump the release if you make any changes to the spec file :)
And I am not going to do the formal review (as I never did one). Just trying to write some script which will do some basic review.

Comment 6 Peter Robinson 2008-08-26 11:18:25 UTC
Updated to the latest version. Build tested in mock

SPEC: http://pbrobinson.fedorapeople.org/tangogps.spec
SRPM: http://pbrobinson.fedorapeople.org/tangogps-0.9.3-1.fc10.src.rpm

Comment 7 Peter Robinson 2008-08-27 18:57:12 UTC
Hi Debarshi,

Are you planning on reviewing this package?

Regards,
Peter

Comment 8 Debarshi Ray 2008-08-29 18:53:47 UTC
This package ships locale files. It manages to build on Koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=793776) since gettext was not mentioned as a BuildRequires. When gettext is present the build fails:

[...]
Checking for unpackaged file(s): /usr/lib/rpm/check-files /var/tmp/tangogps-0.9.3-1.fc8-root-rishi
error: Installed (but unpackaged) file(s) found:
   /usr/share/locale/cs/LC_MESSAGES/tangogps.mo
   /usr/share/locale/de/LC_MESSAGES/tangogps.mo
   /usr/share/locale/fi/LC_MESSAGES/tangogps.mo
   /usr/share/locale/ru/LC_MESSAGES/tangogps.mo


RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/share/locale/cs/LC_MESSAGES/tangogps.mo
   /usr/share/locale/de/LC_MESSAGES/tangogps.mo
   /usr/share/locale/fi/LC_MESSAGES/tangogps.mo
   /usr/share/locale/ru/LC_MESSAGES/tangogps.mo

Consult https://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files

Comment 9 Debarshi Ray 2008-08-29 18:55:20 UTC
On Fedora 8, which is still supported, libcurl-devel does not exist. So if you want this to be available on Fedora 8, you would need to do something like this:

%if 0%{?fc8}
BuildRequires: curl-devel
%else
BuildRequires: libcurl-devel
%endif

Comment 10 Debarshi Ray 2008-08-29 18:55:50 UTC
(In reply to comment #7)
> Hi Debarshi,
> 
> Are you planning on reviewing this package?
> 
> Regards,
> Peter

Please fix these issues and I will do a complete review.

Comment 11 Peter Robinson 2008-08-29 21:55:34 UTC
There's a new version with the gettext fixes in place. New spec in same location, srpm here http://pbrobinson.fedorapeople.org/tangogps-0.9.3-2.fc9.src.rpm

I wasn't planning on supporting Fedora 8 as it will be retired shortly after Fedora 10 is released which isn't far off. If there is demand I may add it later.

Comment 12 Debarshi Ray 2008-09-01 04:19:30 UTC
Created attachment 315455 [details]
tangogps.desktop fix

MUST Items: 

xx - rpmlint is unclean on RPM
    + [rishi@freebook x86_64]$ rpmlint tangogps-0.9.3-2.fc9.x86_64.rpm 
      tangogps.x86_64: W: summary-not-capitalized tangoGPS is a lightweight and fast mapping application
      tangogps.x86_64: E: invalid-desktopfile /usr/share/applications/tangogps.desktop
      1 packages and 0 specfiles checked; 1 errors, 1 warnings.
      [rishi@freebook x86_64]$ 
    + rpmlint is unclean on SRPM
      [rishi@freebook SRPMS]$ rpmlint tangogps-0.9.3-2.fc9.src.rpm 
      tangogps.src: W: summary-not-capitalized tangoGPS is a lightweight and fast mapping application
      1 packages and 0 specfiles checked; 0 errors, 1 warnings.
      [rishi@freebook SRPMS]$ 

OK - follows Naming Guidelines
OK - spec file is named as %{name}.spec

xx - package does not meet Packaging Guidelines
    + To work around the rpmlint warnings you could consider borrowing the
      summary used by Debian (http://packages.debian.org/lenny/tangogps):
      "GTK+ mapping and GPS application"
    + Even though Fedora does not use the Group tag, please consider a more
      suitable value for the sake of perfection.
    + To preserve timestamps you could consider using:
      make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT

OK - license meets Licensing Guidelines
OK - license field meets actual license
OK - upstream license file included in %doc
OK - spec file uses American English
OK - spec file is legible
OK - sources match upstream sources
OK - package builds successfully
OK - ExcludeArch not needed

OK - build dependencies correctly listed
    + The 'BuildRequires: glib2-devel' is redundant since
      'BuildRequires: gtk2-devel' will drag it in as well.

OK - locales handled properly
OK - no shared libraries
OK - package is not relocatable
OK - file and directory ownership
OK - no duplicates in %file
OK - file permissions set properly
OK - %clean present
OK - macros used consistently
OK - contains code and permissable content
OK - -doc is not needed
OK - contents of %doc does not affect the runtime
OK - no header files
OK - no static libraries
OK - no pkgconfig files
OK - no library files
OK - -devel is not needed
OK - no libtool archives

xx - %{name}.desktop file is invalid and not properly installed
    + The .desktop file does not meet the specification. You can use
      desktop-file-validate to have a look at the problems. Please consider the
      attached patch as a fix for these issues.
    + If the package installs a .desktop file, then desktop-file-install must
      be run. See
      https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files Since
      upstream does not provide a vendor ID, you should use "fedora" as the
      value.

OK - does not own files or directories owned by other packages
OK - buildroot correctly prepped
OK - all file names valid UTF-8

SHOULD Items:

OK - upstream provides license text
xx - no translations for description and summary
OK - package builds in mock successfully
OK - package builds on all supported architectures

xx - package will not function as expected
    + Looking at the GUI, it seems that a GPS daemon is necessary during
      runtime. Please add 'Requires: gpsd' to take care of this.

OK - scriptlets are not needed
OK - subpackages are not needed
OK - no pkgconfig files
OK - no file dependencies

Comment 13 Peter Robinson 2008-09-01 10:16:51 UTC
OK. New version uploaded. SPEC in same location, SRPM here
http://pbrobinson.fedorapeople.org/tangogps-0.9.3-3.fc9.src.rpm

Comment 14 Debarshi Ray 2008-09-01 18:53:07 UTC
Please consider submitting your tangogps.desktop fixes upstream.

+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+

Comment 15 Peter Robinson 2008-09-01 19:07:54 UTC
New Package CVS Request
=======================
Package Name: tangogps
Short Description: GTK+ mapping and GPS application
Owners: pbrobinson
Branches: F-9
InitialCC: pbrobinson
Cvsextras Commits: yes

Comment 16 Peter Robinson 2008-09-01 19:10:25 UTC
Thanks, submitting the .desktop upstream is on my todo list. I just need to work out where its generated and create a full patch.

Comment 17 Kevin Fenzi 2008-09-01 21:15:49 UTC
cvs done.

Comment 18 Peter Robinson 2008-09-01 23:20:06 UTC
Thanks for your review. Commited to cvs and built in koji. desktop file changes also submitted upstream and acked for the next release.


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