-
Notifications
You must be signed in to change notification settings - Fork 5
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 Notes (Hao Ye, @ha0ye) #54
Comments
Updated citations for papers that cite this tool with commit bcc79b09 |
Hi Hao, I used issue #57 to keep track of all the functionality reviews (thank you for those) and have, I think, addressed all of them. I'm going to address the others as I go, here. I will try to minimize the number of separate comments so you don't get a ton of notifications. If you don't mind, for my own record-keeping and issue tracking purposes, would you mind checking off the sections that you feel are properly addressed? You could do it here or in the JOSS review or both. General checks
Functionality
DocumentationExample usage
Functionality documentation
Automated testsI still need to work on these. Overlap with @tbrown122387 's comment and definitely something that I need to work on. There are Shiny-specific tests but I think the most important ones are for the embedded functions for population dynamics, etc. Community guidelines
Software paperState of the field
|
Hi @ha0ye -- did you get a chance to look at these revisions yet? Normally I would not bug you but I have a meeting coming up where I'm presenting the package and would like to be able to provide them with a status update. Thank you! |
Yep, sorry. Had a few more functional notes, which I edited into the original post above. |
Hmm, maybe I was thinking of the PBR & PBR calculator tab? Looks good to me, though! |
Ah, I see what you mean. I added that little table to the PBR tab and put in a translation for it. Thanks! I'm going to close this issue now if that's cool with you! |
I'm collecting all the details for my review in this one issue. @mcsiple or another maintainer may consider splitting specific action items into their own separate issues for better task management.
Note: many of these are suggestions for improvements, and in some cases are not actual obstacles for passing the review process at JOSS. These items are indicated by
[~]
rather than an empty box[ ]
. I've also excised all the checked boxes from my review over in openjournals/joss-reviews#3888General checks
Contribution and authorship: Has the submitting author (@mcsiple) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
DESCRIPTION
should be updated to be in line with the author list and version number in CITATION.CFF.Functionality
Installation: Does installation proceed as outlined in the documentation?
Could this be added to
.Rbuildignore
? I'm not sure if it is present to help with running the shiny app as a developer, and would be safe to exclude from the built package for end-users.Functionality: Have the functional claims of the software been confirmed?
arrange
needs to be added to the functions to importFrom dplyr on line 15 of17_plot_proj.R
and line 20 of18_multiplot_proj.R
dynamics()
have separate arguments for the life history parameters, whereas inprojections()
, they're given as a named list. I think this could be made more consistent, and that the list structure is probably better as it is easier to refer to as a single object to pass to multiple functions.Functionality: Have the functional claims of the software been confirmed?
DESCRIPTION
has a duplicatedLazyData
line that is preventing building the package.dplyr::arrange(dplyr::desc(name, sim))
which produces an error. I think it should be something likedplyr::arrange(dplyr::desc(name), dplyr::desc(sim))
Documentation
Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support
Software paper
Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
State of the field: Do the authors describe how this software compares to other commonly-used packages?
References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
The text was updated successfully, but these errors were encountered: