https://bugzilla.redhat.com/show_bug.cgi?id=1926697



--- Comment #9 from jiri vanek <[email protected]> ---
hi!

The link to specfile is broken, but NVM, was easy to find.
thanx for update, the specfile and launcher went  a bit wild, before
continuinfg with review, many nits:

BuildRequires:  java-1.8.0-openjdk
 -> This is wrong. Should be just java-devel. Please consult java packagig
guidelines


sed -i "s|ln -sv|cp -r|g" mvngen.sh
 -> Good idea! However, why "cp -r?" Are there really some directories linked?


%files
%license LICENSE
%doc README.md
/usr/share/java/openjdk-asmtools/*
/usr/share/javadoc/openjdk-asmtools/*
/usr/share/maven-metadata/*
/usr/share/maven-poms/openjdk-asmtools/*
%attr(755, root, -) %{_bindir}/openjdk-asmtools

-> I really od not like aserixes here. Please enumerate what can be enumerated
-> You should not belistin here anything except the last one andmaybe docs and
license. All others *have to be * gatehered by mvn_isntall and thus used via
"-f .mfiles"



The launcher is complete mess :)) Please remove everythinf redundant.
Not, that usage of export _prefer_jre=false is superwrong.
-Djdk.attach.allowAttachSelf=true used without justification is secuirty hole.
Minimal:

#!/usr/bin/bash
set -e
# JPackage Project <http://www.jpackage.org/>
# Source functions library
. /usr/share/java-utils/java-functions
MAIN_CLASS=org.openjdk.asmtools.Main
_set_java_home
set_jvm
set_classpath openjdk-asmtools
run   "$@"

Should do. But I had not tried.



Please. for %{_bindir}/openjdk-asmtools add and isntall manpage. It is simple.
Use output from %{_bindir}/openjdk-asmtools -h . Again, you can inspire in
java-runtime-decompiler.


But looks good. Nice job! Looking forward for this pacakge.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to