Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSS review #85

Open
arcuri82 opened this issue Feb 18, 2021 · 1 comment
Open

JOSS review #85

arcuri82 opened this issue Feb 18, 2021 · 1 comment

Comments

@arcuri82
Copy link

Hi,

First set of comments for openjournals/joss-reviews#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

@ayush-1506
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants