[Bug 508351] Review Request: josm - java openstreetmap editor

[bugzilla at redhat.com]

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=508351





--- Comment #13 from Andrea Musuruane <musuruan at gmail.com>  2009-08-17 06:24:56 EDT ---
* To be consistent with other distributions (Debian ATM) you should use 0.0
instead of 0 as version. This is not mandatory though.

* Release tag is still wrong. It should be 0.X.1788svn%{?dist} and not
1.1788svn%{?dist}. The first 0 indicate that this is a pre-release version. The
X is an incremented number (start at 1) you should bump at *each* new release.
1788svn is the svn version of the checkout you used.

* The version-release couple in the changelog is not consistent (0-1788svn)
with the one declared at the beginning of the spec file. Always run rpmlint on
the rpm produced by rpmbuild. Errors like this one can be easily caught in this
way.

* All patches should have an upstream bug link or comment
https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

In general, it is a good practise to state what a patch does.

* Description can be improved. The following is from the Debian package:
JOSM is an editor for OpenStreetMap (OSM) written in Java.
The current version supports stand alone GPX tracks, GPX track data
from OSM database and existing nodes, line segments and metadata tags
from the OSM database.

OpenStreetMap is a project aimed squarely at creating and providing
free geographic data such as street maps to anyone who wants them. 
The project was started because most maps you think of as free actually
have legal or technical restrictions on their use, holding back people
from using them in creative, productive or unexpected ways.

* Adapt this Debian patch not to include the dependencies into the target jar
file
http://patch-tracking.debian.net/patch/series/view/josm/0.0.svn1529-1/10_build

* After that add gettext-commons and metadata-extractor to Requires.

* Please remove junk from the SPEC file. Like:
#find . -name 'jfcunit.jar' -exec rm -f '{}' \;
#find . -name 'josm-translation.jar' -exec rm -f '{}' \;
#find . -name 'metadata-extractor-2.3.1-nosun.jar' -exec rm -f '{}' \;

* I have some doubt about the following:
install -p -m 644 dist/%{name}-custom.jar
$RPM_BUILD_ROOT%{_javadir}/%{name}-%{version}.jar

%{name}-%{version}.jar will be "josm-0" for a long time. I wonder if you should
call it like upstream "josm-snapshot-svn_version". Maybe some more experienced
java packager should help here :)

* You should add an alias for josm.jar and use it to run the program without
having to edit run script at each release.

* Desktop file category you use is not right. "Network" is used by a "Network
application such as a web browser":

http://standards.freedesktop.org/menu-spec/latest/apa.html

You should use "Categories=Education;Science;Geoscience;"

* You should also add a comment line in the desktop file, like:
Comment=Editor for OpenStreetMap.org

This is not mandatory but it could help the final user.

* A man page is available from Debain. Please include it:
http://patch-tracking.debian.net/patch/debianonly/view/josm/0.0.svn1529-1

* In the generate-tarball script, please:
- clean up temporary directories at the end of the script.
- you could reuse the same tarball in more then one Fedora release therefore do
not use Fedora tags in the filename. Call it something like
"josm-0.0.svnXXXX.tar.gz". You can also generate the filename based on
$JOSM_SVN_TAG. This means you can drop the $NAME_VERSION and $RELEASE
variables.
- are you sure plugins are needed? Debian do not check them out.
- you could use "svn export" and not remove .svn directories later.
- instead of creating a i18n directory and moving into it, you could do it all
in one line, specifying i18n as the destination directory instead of the
current directory ".". The same would apply to plugins if they are required.
- do not bother to remove macosx directory. Moreover, it can be useful. It has
an .icns file with icons at different resolutions. These can be used instead of
a single 128x128 icon. The libicns-utils can be used to extract them.
- IMHO it would be better to get rid of src/org/apache in the %prep section of
the SPEC file.

* Add the following files in %doc:
Contribution gpl-2.0.txt

gpl-2.0.txt is required because of the text of the license must be included in
%doc:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.