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]: highlightHTML: CSS Formatting of R Markdown Documents #185

Closed
whedon opened this issue Feb 21, 2017 · 28 comments

Comments

Projects
None yet
6 participants
@whedon
Copy link
Collaborator

commented Feb 21, 2017

Submitting author: @lebebr01 (Brandon LeBeau)
Repository: https://github.com/lebebr01/highlightHTML
Version: v 0.1.3
Editor: @acabunoc
Reviewer: @daattali
Archive: 10.5281/zenodo.1146049

Status

status

Status badge code:

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

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 questions

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

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: Does the release version given match the GitHub release (v 0.1.3)?
  • Authorship: Has the submitting author (@lebebr01) made major contributions to the software?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: Have any performance claims of the software been confirmed?

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 21, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @desilinguist 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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS 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
@desilinguist

This comment has been minimized.

Copy link

commented Mar 3, 2017

It would be nice if there was some example included in the repository and in the README that made it easy to try out this package. vignette(package='highlightHTML') is also not useful since there are no vignettes. What does the tags argument take in the highlight_html() function? The README just says that it takes a "Vector of CSS code" which is not very helpful.

It'd be really helpful if a usage example can be added so I can continue the review. Thanks!

@lebebr01

This comment has been minimized.

Copy link

commented Mar 6, 2017

Thanks for the comments. I agree, the README could have been better. This has been updated to reflect three things, installation is focused more on the CRAN version. Secondly, an example has been pulled from the vignette to remove the vague line about the tags argument. Third, description about installing the vignettes from the development version was added as well.

A second note, there is a vignette for this package, but these may not install directly from the development version on GitHub. You can view it directly on GitHub here: https://github.com/lebebr01/highlightHTML/tree/master/vignettes

You could also install from GitHub passing the build_vignettes argument: devtools::install_github('lebebr01/highlightHTML', build_vignettes = TRUE).

@arfon

This comment has been minimized.

Copy link
Member

commented Jun 5, 2017

👋 @desilinguist - does @lebebr01's last comment help here?

@lebebr01

This comment has been minimized.

Copy link

commented Aug 9, 2017

@arfon and @desilinguist, I wanted to check in to see if there is any additional information I can provide to help with this? Are there still issues viewing the vignette or is the README example too simple to be helpful?

@desilinguist

This comment has been minimized.

Copy link

commented Aug 9, 2017

Sorry, I have been swamped with other things. LGTM 👍

@arfon

This comment has been minimized.

Copy link
Member

commented Aug 9, 2017

Sorry, I have been swamped with other things. LGTM 👍

@desilinguist - thanks for getting back to us on this. There is still a substantial number of unchecked items above. Would you be able to update the checklist?

@acabunoc

This comment has been minimized.

Copy link

commented Sep 4, 2017

Pinging @desilinguist, have you had time to go through the checklist?

Let me know if you're too busy and I can look for another reviewer 😄

@desilinguist

This comment has been minimized.

Copy link

commented Sep 4, 2017

@lebebr01

This comment has been minimized.

Copy link

commented Sep 8, 2017

Thank you for your time @desilinguist.

Related to another reviewer, I had suggested @daattali during the pre-review stage. Perhaps he would have availability and be willing now.

@daattali

This comment has been minimized.

Copy link

commented Sep 8, 2017

If you are having trouble finding a reviewer, I can be a reviewer

@acabunoc

This comment has been minimized.

Copy link

commented Sep 8, 2017

Thanks so much @daattali! I'll go ahead and assign you as reviewed -- really appreciate your help

@acabunoc

This comment has been minimized.

Copy link

commented Sep 8, 2017

@whedon assign @daattali as reviewer

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 8, 2017

OK, the reviewer is @daattali

@daattali

This comment has been minimized.

Copy link

commented Sep 9, 2017

I haven't done a review in a couple years, I forget how they work. Where are the exact instructions?

@acabunoc acabunoc assigned daattali and unassigned acabunoc and daattali Sep 9, 2017

@acabunoc

This comment has been minimized.

Copy link

commented Sep 9, 2017

@daattali here are the reviewer guideline: http://joss.theoj.org/about#reviewer_guidelines

But mostly, it's going through the checklist at the top of this issue.

Thanks again! 🎉 🎈 🍰

@daattali

This comment has been minimized.

Copy link

commented Oct 18, 2017

Review submitted lebebr01/highlightHTML#5

@lebebr01

This comment has been minimized.

Copy link

commented Oct 23, 2017

Thank you, appreciate the review and thoughtful comments. I'll work through these and post a response/changes in the coming weeks.

@lebebr01

This comment has been minimized.

Copy link

commented Nov 30, 2017

Thanks again @daattali for the review. I posted my comments on edits made here: lebebr01/highlightHTML#5. As mentioned in that issue, most of the changes I grouped into a single commit: lebebr01/highlightHTML@fc1cf86.

I'd be very interested on your thoughts on the changes. Thanks again in advance.

@arfon

This comment has been minimized.

Copy link
Member

commented Dec 18, 2017

@daattali - friendly reminder to take another look at this when you get a chance.

@daattali

This comment has been minimized.

Copy link

commented Dec 18, 2017

@daattali

This comment has been minimized.

Copy link

commented Jan 10, 2018

After revisions, the package LGTM

I left a few more small comments for the author to fix, but they are not blocking issues, I would accept the package now

@arfon

This comment has been minimized.

Copy link
Member

commented Jan 10, 2018

@lebebr01 - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

@lebebr01

This comment has been minimized.

Copy link

commented Jan 11, 2018

Great, thanks @arfon. I created an archive of the reviewed software in Zenodo: https://doi.org/10.5281/zenodo.1146049.

Thank you for your assistance and let me know if you need anything further from me.

@arfon arfon added the accepted label Jan 11, 2018

@arfon

This comment has been minimized.

Copy link
Member

commented Jan 11, 2018

@whedon set 10.5281/zenodo.1146049 as archive.

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2018

OK. 10.5281/zenodo.1146049 is the archive.

@arfon

This comment has been minimized.

Copy link
Member

commented Jan 12, 2018

@daattali - many thanks for your review here and to @acabunoc for editing this submission

@lebebr01 - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00185 ⚡️ 🚀 💥

@arfon arfon closed this Jan 12, 2018

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 12, 2018

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

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

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

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 volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html

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.