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: Issues to address #23

Closed
jhollist opened this issue Jun 20, 2019 · 4 comments
Closed

JOSS Review: Issues to address #23

jhollist opened this issue Jun 20, 2019 · 4 comments

Comments

@jhollist
Copy link
Member

Very nice job on the package! I am a big fan of anything that provides direct access, in R, to these kinds of data sources. There are few things to need to be addressed in the JOSS submission, openjournals/joss-reviews#1473, prior to publication. These are listed below.

  • Software Paper, second paragraph of intro, cde got split at the EOL. Probably a rendering thing, but would be best to make sure that doesn't happen.

  • Software Paper, Statement of need: Set up is there, but a clear indication as to why an R package is needed to access the Catchment Data Explorer. A sentence or two in the second paragraph that expands on the need for the cde package should be added. I buy into the need for these types of data access packages (e.g. reproducibility, automation, etc.), just need to see it in writing in the paper.

  • Documentation, Statement of need: Take a look at https://joss.readthedocs.io/en/latest/review_criteria.html#a-statement-of-need for details on what should be included. Similar to the "statement of need" for the Software Paper, a senetence of two should be added to the README that lay out the reason for developing cde.

  • Documentation, Community Guidelines: Wasn't clear from the repo how one should contribute/report bugs, etc. I'd suggest adding a CONTRIBUTING.md and/or adding details about this to the README

  • Documentation, Example Usage: There are two separate links to documentation sites:

    • https://docs.ropensci.org/cde
    • https://ropensci.github.io/cde
      For the most part the appear very similar. I did notice that the first plotting example in the github.io site plots a different plot than the one included in the documentation. Make sure you link to a single site. It may also be useful to provide a section in the README with a link to the documentation sites and a bit of text explaining how and where to find these examples
@karthik
Copy link
Member

karthik commented Jun 26, 2019

For the most part the appear very similar. I did notice that the first plotting example in the github.io site plots a different plot than the one included in the documentation. Make sure you link to a single site. It may also be useful to provide a section in the README with a link to the documentation sites and a bit of text explaining how and where to find these examples

rOpenSci now automatically deploys the docs for all projects. So the first one should be used, not the second.

@karthik
Copy link
Member

karthik commented Jun 26, 2019

I just realized that this package has already been through rOpenSci's software review, so I'm glad most of these comments are about the paper.

Documentation, Community Guidelines: Wasn't clear from the repo how one should contribute/report bugs, etc. I'd suggest adding a CONTRIBUTING.md and/or adding details about this to the README

Can be fixed with usethis::use_tidy_contributing()

@karthik
Copy link
Member

karthik commented Jun 26, 2019

@robbriers
Copy link
Collaborator

OK, been through the comments:

  • Software paper: fixed split of 'cde' in paragraph and expanded on paragraph including Statement of need.
  • Documentation: included same text as for software paper to expand on the need for the package. Also added Contribution page and link in README.
  • Documentation: examples of usage. The new docs.ropensci.org/cde site came online after the submission, hence the confusion of the two docs sites. All links have been updated to the docs site now. The difference between the figures has also been fixed - this was due to a bug in one of the functions which has been fixed (0.4.1 Github release).
    Hope this is OK, but let me know if anything else needs attention.

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

3 participants