[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 #24 from Andrea Musuruane <musuruan at gmail.com>  2010-01-01 12:33:48 EDT ---
Some additional notes.

* source file must not be called josm.tar.gz. It is too generic. It must have
something that identifies the version and the release too.

"When pulling from revision control, please remember to use a
Name-version-release compatible with the Version and Release Guidelines. In
particular, check the section on Naming Snapshots."
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

A filename called like "josm-0.0.svn2255.tar.gz" would be a good choice. 

* Source file is not packaged correctly. There should be only one root
directory. i18n directory should be one subdirectory. plugin directory must not
be checked out (see comment #22).

* package "manual" is not created. There is no file list. I don't know if this
is an omission or manual is not required.

* I editing/adding the following properties:

version.entry.commit.revision
version.entry.commit.date

in patch0 and patch3 could be avoided by creating a REVISION file in the
expected format (placing it among SOURCEs, with a name like josm-REVISION, and
then copying it in the builddir in %prep) or using sed appropriately in %prep.
In this way you should update these two patches much less often and not at each
upstream release.

BTW, the rest of both patches could be merged since the purpose is the same.

* patch1 is not required. You already use build-classpath in %prep. Another,
alternative, and perhaps more elegant way, to solve this issue is to use
build-jar-repository:

http://fedoraproject.org/wiki/Packaging:Java#build-jar-repository

*  This line:

ln -s $(build-classpath ant) lib

is not needed and wrong. There is no need to put ant in that dir.

* Patch2 should not needed. 

* There is a cleaner way to do the following:

install -p %SOURCE1 %{buildroot}%{_bindir}/%{name}
chmod +x %{buildroot}%{_bindir}/%{name}

just in one line:

install -p -m 755 %SOURCE1 %{buildroot}%{_bindir}/%{name}

-- 
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.