ayush-1506
commented
3 years ago Thanks for the review, @arcuri82 . Will make the changes you pointed out. I've also decided to trim the paper and change the style a bit, will update once that's done.
Open arcuri82 opened 3 years ago
ayush-1506
commented
3 years ago Thanks for the review, @arcuri82 . Will make the changes you pointed out. I've also decided to trim the paper and change the style a bit, will update once that's done.
Hi,
First set of comments for https://github.com/openjournals/joss-reviews/issues/2972
Regarding the paper:
can this submission be considered a "research software"? I see 3 cited papers in the references, but none of the authors? Has this software been used in any published research work?
"Our approach is very different: we treat this as a statistical problem and avoid introducing computationally heavy deep learning into the picture. This makes the solution more explainable and faster to compute". Is there any published empirical evidence to support this claim?
out of the 3 references, only 1 is peer-reviewed. The other 2 are from arXiv. Can you please add some more peer-reviewed references of related work?
In particular, add references to existing open-source software that is doing something similar to your work, or if there is none, explicitly state it.
"Novelties: ADE is written in the Java language that makes it ideal to be used across any platform without changing the underlying code". Why does it matter? You are targeting Linux. Furthermore, a lot of other languages are cross-platform (C#, JavaScript, Python, etc.)
"Novelties: ADE comes batteries-included: One can just build the binary...". Sorry, but "batteries-included" and "just build the binary" do not go well together...
"Novelties: Minimal external dependencies". Why does it matter? You are releasing a self-contained tool, not a library.
Why are the previous claims made under a paragraph called "Novelties"? For me it sounds more like "Features", unless you can clearly compare with the current state-of-the-art.
Regarding the software:
(note: as I do not use Linux, I only compiled your software and run its test cases, plus I read the documentation)
having to build the application, to use it, is very inconvenient. When you make a release, you can add the generated packaged output (eg ade-assembly-1.0.4-bin.tgz) on GitHub, eg at https://github.com/openmainframeproject/ade/releases Right now, you do not have any release. Please make a new one.
"JDK 1.7 or higher". That is not true. I tried with JDK 11, and it does not compile. Note that JDK 9 broke backward compatibility. In your case, it might be a matter of just adding the missing XML libraries that were removed from the JDK. Or simply specify only the JDK versions you have tested.
in README.md: "Please see http://openmainframeproject.github.io/ade/ for documentation on ADE". Before such line, please add a "Documentation" section header, to make it clear
I see you have a .travis.yml file, but no badge on your README.md. Add the status badge. However, notice that Travis is no longer free for open-source software. Unless you are paying for it, I would suggest to switch to either GitHub Actions or CircleCI
you have JUnit tests, and JaCoCo configured. But your test coverage seems quite low. Eg, from ade-core/target/site/jacoco/index.html I can see there is only 7% instruction coverage. You should add some more test cases, and also possibly enable intro-module JaCoCo coverage calculation
publish the coverage results online, and add a badge on README.md. This can be done for example with codecov.io For a concrete example of these badges, see one of my repositories, eg https://github.com/EMResearch/EvoMaster