Bug 454250 - Review Request: tangoGPS - tangoGPS is a lightweight and fast mapping application
Review Request: tangoGPS - tangoGPS is a lightweight and fast mapping applica...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Debarshi Ray
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-07 04:46 EDT by Peter Robinson
Modified: 2008-09-01 19:20 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-01 19:20:06 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
debarshir: fedora‑review+
kevin: fedora‑cvs+


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

  None (edit)
Description Peter Robinson 2008-07-07 04:46:51 EDT
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 15:53:26 EDT
Created attachment 314134 [details]
Mock build log
Comment 2 kushaldas@gmail.com 2008-08-12 15:54:31 EDT
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 16:12:25 EDT
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 16:20:47 EDT
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 02:04:11 EDT
(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 07:18:25 EDT
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 14:57:12 EDT
Hi Debarshi,

Are you planning on reviewing this package?

Regards,
Peter
Comment 8 Debarshi Ray 2008-08-29 14:53:47 EDT
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 14:55:20 EDT
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 14:55:50 EDT
(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 17:55:34 EDT
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 00:19:30 EDT
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 06:16:51 EDT
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 14:53:07 EDT
Please consider submitting your tangogps.desktop fixes upstream.

+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+
Comment 15 Peter Robinson 2008-09-01 15:07:54 EDT
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 15:10:25 EDT
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 17:15:49 EDT
cvs done.
Comment 18 Peter Robinson 2008-09-01 19:20:06 EDT
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.