[Bug 475017] Review Request: l2fprod-common - In JavaSE missing Swing components, inspired from modern user interfaces

[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=475017


Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |mtasaka at ioa.s.u-tokyo.ac.jp
               Flag|needinfo?(red at fedoraproject |fedora-review?
                   |.org)                       |




--- Comment #18 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-04-28 15:07:04 EDT ---
Well, for 7.3-3:

* License
  - Sorry, I missed it however one file is still under
    ASL 2.0
-------------------------------------------------------------
src/tests/com/l2fprod/common/demo/MainUnitTest.java
-------------------------------------------------------------
    Can this file be removed at %prep? (Your spec file
    seems to be using files only under src/java, so
    I guess it is possible)
    ! Note that only removing this file at %prep is enough.
      No need to remove this file from tarball itself.

* CVS directory
  - It is recommended to clean up CVS directory at %prep like:
-------------------------------------------------------------
find . -type d -name CVS | sort -r | xargs rm -rf
-------------------------------------------------------------

* Comments
  - What are the comments like
-------------------------------------------------------------
###############################################################rm -rf docs lib
src/com/l2fprod/common/springrcp
-------------------------------------------------------------
    for? (I guess you just copy-and-pasted mistakenly...)

* Multilue jar files (including symlink)
  - The following guideline seems to be saying that this package
    should create a directory and move all jar files (including symlinks)
    into the directory:
-------------------------------------------------------------
If the number of provided JAR files exceeds two, you must 
place them into a sub-directory. 
-------------------------------------------------------------

And some suggestion about spec file clean up
! Using xargs
  - From the line
-------------------------------------------------------------
javac -d target -cp `build-classpath nachocalendar jcalendar` `find src/java
-name '*.java'`
-------------------------------------------------------------
    build.log shows lots of output messages brought from
    `find src/java -name '*.java'` like:
-------------------------------------------------------------
    46  + javac -d target -cp
/usr/share/java/nachocalendar.jar:/usr/share/java/jcalendar.jar 
src/java/totd/com/l2fprod/common/swing/tips/DefaultTip.java
src/java/totd/com/l2fprod/common/swing/tips/DefaultTipModel.java 
src/java/totd/com/l2fprod/common/swing/tips/TipLoader.java
src/java/totd/com/l2fprod/common/swing/TipModel.java 
.....
-------------------------------------------------------------

    In this case I usually use xargs like:
-------------------------------------------------------------
find src/java -name '*.java' -print0 | xargs -0 javac -d target -cp
`build-classpath nachocalendar jcalendar`
-------------------------------------------------------------
    With this, build.log shows:
-------------------------------------------------------------
    48  + find src/java -name '*.java' -print0
    49  ++ build-classpath nachocalendar jcalendar
    50  + xargs -0 javac -d target -cp
/usr/share/java/nachocalendar.jar:/usr/share/java/jcalendar.jar
    51  src/java/sheet/com/l2fprod/common/beans/BeanUtils.java:41: warning:
non-varargs call of varargs method with inexact argument type for last
parameter;
    52  cast to java.lang.Class for a varargs call
-------------------------------------------------------------
    which is much easier to look at.

! About creating many symlinks manually
  - I think this is error-prone. I guess the following is better:
--------------------------------------------------------------
install -m644 %{name}-%{version}.jar
$RPM_BUILD_ROOT%{_javadir}/%{name}-all-%{version}.jar
ln -s %{name}-all-%{version}.jar $RPM_BUILD_ROOT%{_javadir}/%{name}-all.jar

for dir in src/java/*/
do
 subname=$(basename $dir)
 for exception in \
  demo springrcp
 do
  if [ $subname = $exception ] ; then continue 2 ; fi
 done
 ln -sf %{name}-all-%{version}.jar
$RPM_BUILD_ROOT%{_javadir}/%{name}-${subname}.jar
 ln -sf %{name}-all-%{version}.jar
$RPM_BUILD_ROOT%{_javadir}/%{name}-${subname}-%{version}.jar
done
---------------------------------------------------------------
     Also I would just use "%{_javadir}/%{name}-*.jar" in %files
     (however also see the previous comment about 
    "Multilue jar files (including symlink)".

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