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

[REVIEW]: getTBinR: an R package for accessing and summarising the World Health Organisation Tuberculosis data #1260

Closed
whedon opened this issue Feb 18, 2019 · 48 comments

Comments

Projects
None yet
6 participants
@whedon
Copy link
Collaborator

commented Feb 18, 2019

Submitting author: @seabbs (Sam Abbott)
Repository: https://github.com/seabbs/getTBinR
Version: 0.5.8
Editor: @trallard
Reviewer: @rrrlw, @strengejacke
Archive: 10.5281/zenodo.2547405

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/7c2f7f4ae615e6d70fa2d921b6d784fc"><img src="http://joss.theoj.org/papers/7c2f7f4ae615e6d70fa2d921b6d784fc/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/7c2f7f4ae615e6d70fa2d921b6d784fc/status.svg)](http://joss.theoj.org/papers/7c2f7f4ae615e6d70fa2d921b6d784fc)

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) 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

@rrrlw & @strengejacke, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @trallard know.

Please try and complete your review in the next two weeks

Review checklist for @rrrlw

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Version: 0.5.8
  • Authorship: Has the submitting author (@seabbs) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • 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 function 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

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @strengejacke

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Version: 0.5.8
  • Authorship: Has the submitting author (@seabbs) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • 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 function 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

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2019

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @rrrlw, it looks like you're currently assigned as the reviewer for this paper 🎉.

⭐️ Important ⭐️

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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2019

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 18, 2019

👋 @rrrlw and @strengejacke this is the review issue the latest version of the JOSS paper is just up here ⬆

Please use this issue to conduct your review and discuss any issues/concerns raised during the review process.

If you have any questions at any point, please feel free to ping me

@rrrlw

This comment has been minimized.

Copy link

commented Feb 18, 2019

Minor point: the GitHub README meets the criteria for "Statement of need" in the Documentation section (corresponding box has been checked), however, a slightly more detailed description in the DESCRIPTION file could be helpful for useRs who download the CRAN package w/o looking at the GitHub page.

@rrrlw

This comment has been minimized.

Copy link

commented Feb 18, 2019

Minor point: the Depends, Imports, and Suggests fields in the DESCRIPTION file meet the criteria for "Installation instructions" (corresponding box has been checked), but it is often helpful to include minimum package versions for the Imports and Suggests fields (as you do for the R dependency under the Depends field).

@rrrlw

This comment has been minimized.

Copy link

commented Feb 18, 2019

Quick update: this is an easy submission to review: wonderful & useful software, evidenced by numerous examples, vignettes, sample data, smooth installation, clean pkgdown website; overall this looks like a mature package with a good number of users that will be useful to researchers.

There are a couple of minor points (as stated above + minor LICENSE point opened as issue in repo) that could improve the package (at the author's discretion); I will spend some more time taking a more detailed look at the vignettes and package code to confirm the function claims of the software are met. Barring any major surprises, I foresee the last couple of boxes being checked soon and my review being complete.

Great work, @seabbs!

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 19, 2019

Hi @rrrlw,

Thanks for reviewing.

Do you have a suggestion for how to establish the minimum package versions?

Barring an automated approach, the solutions I see is:

  1. Testing the R version given as the minimum and testing the packages available at that time (using rocker docker containers to get package versions).
  2. Rolling forward the package versions until all tests pass.

Any other solution appreciated as this seems a little tedious!

Modified the license and description as suggested.

Sam

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 19, 2019

Hi @rrrlw

Just an update on dep versions. I have updated the R dependency to be 3.1.2 rather than 2.1. This is the earliest R version that I am actively testing on travis so will definitely work. 2.1 was set by devtools (I think) and I don't really have a guarantee that it works (:().

For package dependencies I have used usethis::use_tidy_versions() to set package deps based on what my development environment has loaded. This looks to be what dplyr is doing. I have also added this to the Makefile so this will update as I do further work.

Let me know if you think this makes sense.

Sam

@rrrlw

This comment has been minimized.

Copy link

commented Feb 19, 2019

The R dependency is always the trickiest for me as well because updating R can be a pain for many useRs; I think you have selected a reasonable minimum (although I think 2.1 was also acceptable). Given that installing/updating a package is generally not as troublesome, your solution of using the versions in your environment is ideal. Thank you for the rapid changes.

@strengejacke

This comment has been minimized.

Copy link

commented Feb 19, 2019

As far as I'm concerned and what I have tested and seen from the package, all remaining issues have been addressed by @seabbs. The package documentation, examples and descriptions are pretty straightforward and clear. The same holds true for the paper. Thus, I recommend this paper for publication in JOSS

@rrrlw

This comment has been minimized.

Copy link

commented Feb 19, 2019

I agree with @strengejacke. Thank you for your contribution, @seabbs.

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 20, 2019

Thank you both (@rrrlw and @strengejacke) for taking the time to review.

Very helpful feedback from you both and a very smooth process.

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 21, 2019

Hi @trallard,

Is there something I need to do now that strengejacke and rrrlw have wrapped up their review?

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Hi all thanks for the time spent in this review, it seems that all the issues have already been dealt with and there are just a few steps between acceptance and now.

@seabbs can you please make sure to complete the following tasks before completing this?

  • make a new release of the package (unless the latest release is up to date with the recent modifications) and add let us know what is the most up to date version
  • upload the revised software to your DOI-granting data/software repository, and post the DOI here
  • double check the submission paper to ensure it is up-to-date (citations are correct, DOIs are added, etc)
@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Will do.

Is this a GitHub release or CRAN release?

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

We normally ask for the GitHub version as this is also the one that will be used in Zenodo, but thinking about it: it would be great to have all versions in sync (GitHub, CRAN, Zenodo) of course if you have the time/capacity to do so

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

I'd rather wait on the CRAN release if possible as my last CRAN release was only a month ago. I also have some features planned that I would like to implement in the next release. The issues above were fairly cosmetic (but very useful) so package functionality is unchanged between CRAN and GitHub. If you think the benefits from syncing up outweigh above then happy to release today.

In terms of the Zenodo DOI is it the one for the specific release or is it the one that links to the latest release?

@strengejacke

This comment has been minimized.

Copy link

commented Feb 26, 2019

Usually, you create a new Zenodo-DOI on the latest GitHub version, i.e. the version after review. That's the DOI for the JOSS article.

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Sorry if I am being thick.

To clarify a DOI that points to the specific version of the package that was reviewed here - rather than a DOI that points to the latest Zenodo release?

@strengejacke

This comment has been minimized.

Copy link

commented Feb 26, 2019

You have made some revisions (during the review process), so you should change your package version number and create a new DOI, so the Zenodo-archive contains all latest changes after review.

That is the required DOI.

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Hey @seabbs no worries this can be a bit confusing let me go over what you'd need to do:

  1. Create a new release (tag) on GitHub of the package
  2. Create a new Zenodo-DOI pointing to the newly created release (if you already have the code archived in Zenodo it should be a matter of adding the new version)
  3. Copy the package DOI to this thread
@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Yes I totally agree on that - making a release now.

My point is that Zenodo has two DOI types. One points to a specific release and one tracks the latest release.

From what you are both saying the DOI I need is the one that links to the reviewed package. Obviously, this is good as it shows what was reviewed. The downside is that it will be out of date as soon as there is a new release.

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Sorry I just corrected my previous comment: share the DOI that points to the latest release (that should include the reviewed version)

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Checklist:

  • make a new release of the package (unless the latest release is up to date with the recent modifications) and add let us know what is the most up to date version
  • upload the revised software to your DOI-granting data/software repository, and post the DOI here
  • double check the submission paper to ensure it is up-to-date (citations are correct, DOIs are added, etc)

Changes:

  • Updated paper with correct latest release package DOI rather than version DOI
  • Added Sciences to department naming in paper.
  • Updated package version to 0.5.8 and released on GitHub
  • Uploaded to Zenodo (via GitHub)
  • DOI (to latest release): 10.5281/zenodo.2547405
@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@whedon set 10.5281/zenodo.2547405 as archive

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@whedon set 10.5281/zenodo.2547405 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

OK. 10.5281/zenodo.2547405 is the archive.

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@whedon set 0.5.8 as version

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

OK. 0.5.8 is the version.

@trallard trallard added the accepted label Feb 26, 2019

@trallard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

@seabbs all looks good to me so I am going to proceed for acceptance

@rrrlw and @strengejacke thanks a lot for your time and valuable contribution to JOSS as reviewers for this submission 🙌🏻

@openjournals/joss-eics this submission has been accepted and is ready to be published 🎉🙌

@seabbs

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2019

Thanks, @trallard for overseeing this.

Thanks again to @rrrlw and @strengejacke for the great review.

Great painless experience. It would be great if traditional journals moved towards this kind of model!

@danielskatz

This comment has been minimized.

Copy link

commented Feb 26, 2019

Thanks all! Processing for acceptance will happen later today/tonight.

@danielskatz

This comment has been minimized.

Copy link

commented Feb 26, 2019

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Check final proof 👉 openjournals/joss-papers#521

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#521, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019


OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1515/9783038216162.117 may be missing for title: R: a language and environment for statistical computing

INVALID DOIs

- None
@danielskatz

This comment has been minimized.

Copy link

commented Feb 26, 2019

@whedon accept deposit=true

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Doing it live! Attempting automated processing of paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 openjournals/joss-papers#522
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01260
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.01260/status.svg)](https://doi.org/10.21105/joss.01260)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01260">
  <img src="http://joss.theoj.org/papers/10.21105/joss.01260/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01260/status.svg
   :target: https://doi.org/10.21105/joss.01260

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

@danielskatz

This comment has been minimized.

Copy link

commented Feb 26, 2019

Thanks again to @trallard for editing, and @rrrlw and @strengejacke for reviewing!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.