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=619928 Bug 619928 depends on bug 583643, which changed state. Bug 583643 Summary: Review Request: tigase-xmltools - Tigase XML Tools https://bugzilla.redhat.com/show_bug.cgi?id=583643 What |Old Value |New Value ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution| |ERRATA --- Comment #6 from Matej Cepl <mcepl@xxxxxxxxxx> 2010-08-07 00:26:36 EDT --- (In reply to comment #5) > In current version of package you may safely drop them as they are. I did. There is now in %prep # to make delete verbose don't use -delete find . -name \*.jar -exec rm -v '{}' \; and jars are not installed. > Alternatively, you may choose to package the tools from src/main/groovy if you > find them useful. If you choose to do so, you can decide whether to build them > into .jars at package build time (preferred) and add groovy BR, or leave them > as they are (the source form) and drag groovy interpreter as binary package > dependency. The problem I have is that I would like to have tigase-server working on EL5 and there doesn't seem to be groovy on neither EL6 nor EL5 (actually to be sure it works on EL5, I am doing all packaging work on EL5 virtual machine). For now, I have installed groovy scripts into %doc and if anybody wants to have groovy scripts he must to hack groovy support himself. I hope that could be an acceptable solution. > 4.) This is not a wise thing to do: "ant -Dlibs=%{_javadir}" > > Javadir can grow too big and apart from java being slow, you easily loose track > of jars you depend on. A better solution would be to drop the -Dlibs argument > and prepend the call to ant with something like: > > build-jar-repository libs tigase-utils tigase-xmltools Wov! That's a great tool ... I haven't knew about it. Fixed. > 5.) Please comment on why are you missing the %check section with ant > run-unittest command. (It seems to me that it is missing dependency on wttols > which provides the unitgen task, right?) Yes, and I've made a comment to the %check section > 6.) The startup script does not seem to work > > JAVA_HOME makes little sense in jpackage environment. > Please drop the check, or hardcode the value to /usr/lib/jvm. I don't think it has anything to do with JAVA_HOME (which is not set) but with asking for groovy* jars. I had them installed from my previous experiments, but they are not packaged anymore. Fixed. > 7.) /etc/tigase/tigase.conf contains both user modifiable and > non-user-modifiable settings. > > Therefore, in case the user e.g. tunes the VM for higher heap size, and > another version of the package adds new jar to the default classpath, an > update results in non-functional configuration. Please consider splitting > the file into two. Created /etc/tigase/tigase.defaults which is sourced by /etc/tigase/tigase.conf, but it shouldn't be changed by the sysadmin > 8.) Why does /etc/tigase/database go into /etc? You are right. I was on the fence about switching it back to /usr/share/tigase/database, which I did now. > Hopefully that's all. /me hopes too :) SPEC file is http://mcepl.fedorapeople.org/rpms/tigase-server.spec src.rpm is http://mcepl.fedorapeople.org/rpms/tigase-server-5.0.0-0.3.20100527svn.src.rpm -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review