[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#575850: RFS: libspring-webflow-2.0-java



Hi again Damien, thanks for the review.
Please see my comments below.

On Wed, Mar 31, 2010 at 4:12 PM, Damien Raude-Morvan <drazzib@debian.org> wrote:
> Hi Miguel,
>
> Here we go for libspring-webflow-2.0-java review :
>
> - debian/patches/02_fix_compilation.diff
> You should try to forward your patch upstream (maybe they'll be please to
> deliver you a parser compliant with OGNL version in debian)

Ok, I will try to find how to send that patch to upstream.

> - debian/README.Debian-source
> Should be renamed to debian/README.source (as per Debian Policy § 4.14)

Renamed. Done.

> - debian/control:
>  * libspring-js-2.0-java short description seems a bit too generic
> "Javascript abstraction framework". What about "server-side Javascript
> abstraction framework" ?
>  * libspring-webflow-2.0-java:
>  junit seems removable
>  libhibernate3-java is twice + should be Recommends

Done. I updated the short description for libspring-js-2.0 and fixed the
Depends/Recommends for this package.

> - projects/spring-js/src/main/resources/META-INF/dojo/
> Seems already available inside libjs-dojo-core package
> 1) you can of course ship it inside original tarball (after all it's DFSG-
> compliant)
> 2) you should try to use existing JS and don't embedded it in JAR file. I
> don't exactly know how Spring.js can handle this :/ As a general guideline, we
> should try to avoid embedded code copies.
> Same apply for META-INF/dijix/ (in libjs-dojo-dijix package)

Yes, I forgot that those Javascript frameworks are already available in Debian.
I removed those embedded code copies. Thanks for pointing this out.

> - notice.txt (install)
> There is no need to install this file as we already ship it in source form
> (orig tarball) and copyright notice are provided by "copyright" file in each
> binary package (compliant with Apache License 2.0 §4d)

Done. I removed notice.txt from the binary packages.

> - readme.txt
> Debhelper already take care of changelog.txt, but maybe you should install
> readme.txt too ?

Done. I included that file in the binary packages.

> - docs/spring-webflow-reference/
> As you already strip Javadoc from source package, you may want to remove all
> this. (As you may know, docbook source is here : projects/spring-webflow-
> reference/)

I prefer to not remove those docs yet. The reference manual can be generated
from the source but the ant target provided by upstream to build that
documentation
depends on some libraries not available in Debian yet.

> Embedded code copies is the only blocking issue for me to upload your package.
> Can you provide me some feedback about this ?

I removed those embedded code copies from spring-js jar file.
The right thing should be Recommends libjs-dojo-core and libjs-dojo-dijit
and document this change from upstream in README.Debian.

I uploaded a new version to mentors and pushed the changes to the git repo.
Let me know if more changes are needed.
Cheers,

- URL: http://mentors.debian.net/debian/pool/main/l/libspring-webflow-2.0-java
- Source repository: deb-src http://mentors.debian.net/debian unstable
main contrib non-free
- dget http://mentors.debian.net/debian/pool/main/l/libspring-webflow-2.0-java/libspring-webflow-2.0-java_2.0.8.RELEASE-1.dsc
- Vcs-Git: git://git.debian.org/pkg-java/libspring-webflow-2.0-java.git


-- 
Miguel Landaeta, miguel at miguel.cc
secure email with PGP 0x7D8967E9 available at http://keyserver.pgp.com/
"Faith means not wanting to know what is true." -- Nietzsche



Reply to: