Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign up[REVIEW]: f90nml - A Python module for Fortran namelists #1474
Comments
whedon
assigned
danielskatz
May 24, 2019
whedon
added
the
review
label
May 24, 2019
whedon
referenced this issue
May 24, 2019
Closed
[PRE REVIEW]: f90nml - A Python module for Fortran namelists #1472
This comment has been minimized.
This comment has been minimized.
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @zbeekman, it looks like you're currently assigned as the reviewer for this paper If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews To fix this do the following two things:
For a list of things I can do to help you, just type:
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
May 24, 2019
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
May 24, 2019
As stated in the first comment in this issue, please carry out your review in this issue by updating the your checklist (above). Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.) Let me know if you have any problems or questions |
This comment has been minimized.
This comment has been minimized.
I can't seem to tick any check boxes |
This comment has been minimized.
This comment has been minimized.
Works for me. Of course, I first started checking @zbeekman 's boxes ... But I then unchecked them. |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
May 24, 2019
Quoting the comment above:
I'm sure the first is ok - can you make sure you did the second? |
This comment has been minimized.
This comment has been minimized.
I'd accepted already for a previous review. It started working just now. Thanks! |
whedon
assigned
tclune and
zbeekman
May 25, 2019
This comment has been minimized.
This comment has been minimized.
I had one minor issue with the installation. 3 different methods are described, but the one that that avoids the requirement for root privileges did not work for me. I have amended a previously closed issue. |
This comment has been minimized.
This comment has been minimized.
My only other negative comments are:
|
This comment has been minimized.
This comment has been minimized.
marshallward
commented
May 31, 2019
Thanks very much for the feedback. On the three points:
|
This comment has been minimized.
This comment has been minimized.
@marshallward Please disregard my API comment for now. I did not see the Makefile and was just staring at the raw files in the ./doc directory. I'm building the documentation and will update my comment. |
This comment has been minimized.
This comment has been minimized.
I retract my comment about the API. Documentation is complete and nicely done. And I thank the author for getting me to actually look at sphinx. I'd heard of it, but never (knowingly) used it before. |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Jun 3, 2019
|
This comment has been minimized.
This comment has been minimized.
Jumping in now. |
Jun 3, 2019
This was referenced
This comment has been minimized.
This comment has been minimized.
Once community/contributing is addressed, I'm happy to check the last checkbox. IMO, the last two issues are not show-stoppers, but I would categorize them as minor changes requested. I could comment on a number of the boxes that I checked during the review process to document my justification and thought process, if that would be useful. @danielskatz please let me know if you think I should do this and whether or not there are any additional tasks for me, beyond checking off the contributing/community box once upstream provides adequate documentation. @marshallward Nice work! |
This comment has been minimized.
This comment has been minimized.
Also, it looks like a lot of climate science tools/packages are using f90nml: https://github.com/marshallward/f90nml/network/dependents very cool! |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Jun 4, 2019
No, you don't need to do this. |
This comment has been minimized.
This comment has been minimized.
@danielskatz My review is finished. I can confirm that the paper and the source code meet the review criteria determined by JOSS, and enthusiastically recommend this paper/package for publication. My only remaining advice to the author is that the reliance of the package on the Otherwise, the package is very well written, performs testing and installation in a very straightforward, pythonic, and canonical way. It includes an extensive automatic test suite with great code coverage, and thorough documentation. If I can be of any additional assistance, please do not hesitate to ask. Thanks! -Izaak "Zaak" Beekman |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Jun 5, 2019
Thanks @zbeekman! |
This comment has been minimized.
This comment has been minimized.
danielskatz
commented
Jun 5, 2019
as advice - if you want to make changes based on this, please do. |
This comment has been minimized.
This comment has been minimized.
marshallward
commented
Jun 5, 2019
Thanks @danielskatz, I am currently working through this and other issues with @zbeekman and @tclune in the GitHub issues cited above, it's been valuable feedback. |
whedon commentedMay 24, 2019
•
edited by zbeekman
Submitting author: @marshallward (Marshall Ward)
Repository: https://github.com/marshallward/f90nml
Version: v1.1
Editor: @danielskatz
Reviewer: @zbeekman, @tclune
Archive: Pending
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@zbeekman & @tclune, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @danielskatz know.
Review checklist for @zbeekman
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @tclune
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?