Joss-reviews: [REVIEW]: ros_control: A generic and simple control framework for ROS

Created on 15 Nov 2017  ·  58Comments  ·  Source: openjournals/joss-reviews

Submitting author: @bmagyar (Bence Magyar)
Repository: https://github.com/ros-controls/joss_paper
Version: 0.12.0
Editor: @danielskatz
Reviewer: @progtologist
Archive: 10.5281/zenodo.1069607

Status

status

Status badge code:

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

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 1 questions

@progtologist, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @danielskatz know.

Conflict of interest

Code of Conduct

General checks

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

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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

Reviewer 2 questions

@miguelprada, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @danielskatz know.

Conflict of interest

Code of Conduct

General checks

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

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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

Most helpful comment

Also, thanks for @miguelprada for his review.

All 58 comments

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @progtologist it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

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

@progtologist & @miguelprada, please use the two checklists above (one for each of you) to carry out your reviews.

An informal guideline is that we would like your review in 2 weeks, but sooner (or later) are also ok.

The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines

Please avoid lengthy details of difficulties in this review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in this review thread. (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.)

Any questions/concerns, please let me know.

Thanks!!

@danielskatz My two first questions already:

First, I might be missing something really obvious here, but I cannot seem to mark the checklist items in the original comment.

Second, the repository listed in the original comment (both in the header and the first item in _general checks_) is to the contents of the paper, not the software itself. Is this ok?

To be clear, I know where to find the software, I just hesitated when I saw the text

Repository: Is the source code for this software available at the repository url?

with a link pointing to a repo containing just the paper.

@migueldvb sorry, I need to assign you to this issue, and am working on that.

regarding the second point, you are correct, and the repo should point up one level, not to the paper.

@arfon, help on both of these points is welcome.

First, I might be missing something really obvious here, but I cannot seem to mark the checklist items in the original comment.

@miguelprada - you should have access now (please accept the invite from the openjournals organization).

regarding the second point, you are correct, and the repo should point up one level, not to the paper.

@bmagyar - could you please advise?

First, I might be missing something really obvious here, but I cannot seem to mark the checklist items in the original comment.

@miguelprada - you should have access now (please accept the invite from the openjournals organization).

Accepted the org invite, but I'm still unable to check the items... :sweat:

May I be doing something wrong? It should be just a matter of clicking on the checkboxes, right? I don't see any other way to edit the original post either.

Accepted the org invite, but I'm still unable to check the items... 😓
Sorry, one more thing. Please accept the invite on this page: https://github.com/openjournals/joss-reviews/invitations

(This usually happens automatically via the API but as we can't handle multiple reviewers automatically yet I have to give you access manually)

Works now. Thanks @arfon!

@danielskatz can you provide your feedback regarding this issue?

As I just wrote in the issue:

JOSS requires a LICENSE or LICENSE.txt file in the root of the repository.

Grade: 1) Accept

All minor issues are now resolved as far as I am concerned. The software is research orientated with wide acceptance in the field and is actively developed and maintained.

Thanks @progtologist !

@miguelprada , how's your review coming along?

I'll issue a grade 2, if only for the missing DOI entries. Will replace it with a 1 once they're in place.

That was quick! Grade: 1 (accept).

Thanks @miguelprada !

@arfon, can I pass this over to you now for the final steps?

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

What would you recommend for gathering up all the package into a tar and submitting for doi?
Zenodo only seems to support repo-wise archiving, figshare seems to be ok with the big tarball.

Should I add a pdf generated by me to the joss-paper repo or will that be generated by them/you?

@arfon @danielskatz

What would you recommend for gathering up all the package into a tar and submitting for doi?
Zenodo only seems to support repo-wise archiving, figshare seems to be ok with the big tarball.

My understanding is that both figshare and Zenodo will let you archive a bunch of zip/tar files with them, Zenodo just has a cleaner/more automated approach if you're archiving a single repository.

Should I add a pdf generated by me to the joss-paper repo or will that be generated by them/you?

That will be done by us. We just just need you to supply us with the archive DOI for the software that has been reviewed as part of this submission.

Thanks for the comments @arfon !

Here is the link to the archive: https://zenodo.org/record/1069607

DOI

@whedon set 10.5281/zenodo.1069607 as archive

OK. 10.5281/zenodo.1069607 is the archive.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #456 with the following error: 

 /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/author.rb:32:in `block in build_affiliation_string': Problem with affiliations for Jonathan Bohren, perhaps the affiliations index need quoting? (RuntimeError)
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/author.rb:31:in `each'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/author.rb:31:in `build_affiliation_string'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/author.rb:11:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:97:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:97:in `block in parse_authors'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:95:in `each'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:95:in `parse_authors'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:76:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/processor.rb:26:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/processor.rb:26:in `set_paper'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/bin/whedon:37:in `prepare'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/bin/whedon:75:in `<top (required)>'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `load'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `<main>'

@bmagyar , can you merge https://github.com/ros-controls/joss_paper/pull/26 please?
Then we'll see if this fixes the problem generating the PDF

Thanks for giving a shot to fix it. It's merged

On Dec 4, 2017 01:17, "Daniel S. Katz" notifications@github.com wrote:

@bmagyar https://github.com/bmagyar , can you merge
ros-controls/joss_paper#26
https://github.com/ros-controls/joss_paper/pull/26 please?
Then we'll see if this fixes the problem generating the PDF


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/456#issuecomment-348837121,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADXH4ScfCLRhvH6YEi4cm9HwAwGEsjO7ks5s80g9gaJpZM4Qe6Oa
.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #456 with the following error: 

 pandoc-citeproc: "stdin" (line 78, column 2):
unexpected "m"
expecting "c", "C", "p", "P", "s" or "S"
CallStack (from HasCallStack):
  error, called at src/Text/CSL/Input/Bibtex.hs:113:32 in pandoc-citeproc-0.10.4-BdOyQb33rzG2TMOLj4Fbp9:Text.CSL.Input.Bibtex
pandoc: Error running filter pandoc-citeproc
Filter returned error status 1
Looks like we failed to compile the PDF

hey @arfon - this is a very painful way to debug markdown/bibtex -> PDF. Maybe we need a locally runnable option as well?

@bmagyar - while the reviewers checked the references box, it looks like there is at least one paper in the bibtex file, the last one (radford2015valkyrie), that has a DOI but where the DOI is not part of the bibtext.

Please make sure all articles that you include in the bibtex that have DOIs have them in their bibtex entries.

Sorry for not catching that one, @danielskatz :sweat:

Sorry for the trouble, indeed it would be nice to have something I could run locally.

I've updated the bibtex with one I took from Wiley directly, they've got DOI and ISSN.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #456 with the following error: 

 pandoc-citeproc: "stdin" (line 78, column 2):
unexpected "m"
expecting "c", "C", "p", "P", "s" or "S"
CallStack (from HasCallStack):
  error, called at src/Text/CSL/Input/Bibtex.hs:113:32 in pandoc-citeproc-0.10.4-BdOyQb33rzG2TMOLj4Fbp9:Text.CSL.Input.Bibtex
pandoc: Error running filter pandoc-citeproc
Filter returned error status 1
Looks like we failed to compile the PDF

hey @arfon - this is a very painful way to debug markdown/bibtex -> PDF. Maybe we need a locally runnable option as well?

Yes, there is a way to run locally. You need to download and install the Whedon gem (or Pandoc etc locally).

In this case, the error is due to missing commas in the bibtex file which I've fixed up here: https://github.com/ros-controls/joss_paper/pull/27

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
https://github.com/openjournals/joss-papers/blob/joss.00456/joss.00456/10.21105.joss.00456.pdf

Hey @arfon

  1. In the PDF, the "repository" link on the left points to the paper repo (https://github.com/ros-controls/joss_paper) rather than the high-level set of repos (https://github.com/ros-controls) I understand why this is the case, but in this unusual paper describing a set of repos, this link should probably be changed manually.
  2. Also in the PDF, the archive link seems wrong - it seems to point to the code repo, not to the zenodo archive...
  • In the PDF, the "repository" link on the left points to the paper repo (https://github.com/ros-controls/joss_paper) rather than the high-level set of repos (https://github.com/ros-controls) I understand why this is the case, but in this unusual paper describing a set of repos, this link should probably be changed manually.

OK, I can fix that up.

Also in the PDF, the archive link seems wrong - it seems to point to the code repo, not to the zenodo archive...

I'm not sure it currently links to anything. This link is only made in the final version of the paper.

Assuming both of the above are fixed up, is this paper good to accept? If so, I can do the final pieces.

let's wait until https://github.com/ros-controls/joss_paper/pull/28 is merged, which fixes some bib issues and also includes a note about a correct URL that @bmagyar needs to find (as I can't find the right one)

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@arfon - any idea what got stuck here? Reticulation of splines is usually such a quick operation...

anyhow, once this works, I think we are good to accept this one.

@arfon - any idea what got stuck here? Reticulation of splines is usually such a quick operation...

I think Heroku might be doing some platform maintenance right now (I'm seeing some weird behaviour elsewhere). I'll do some digging.

@progtologist - many thanks for your review here and to @danielskatz for editing this submission ✨

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

Also, thanks for @miguelprada for his review.

Hello, I am @graiola, one of the authors.
First of all, I would like to thank you all for your efforts in making this publication possible.
By looking at the resulting pdf I noticed that the markdown list has not been correctly transformed (look at the robot section). Is it possible to fix it? Thanks in advance.

@arfon - if you look at https://github.com/ros-controls/joss_paper/blob/master/paper.md you will see a section

Robots using ros_control

that has a list of bullets. In the version at the DOI, "ros_control" is in a too-small font, and in both the version at the DOI and the PDF, the bullets have been lost.

Can you fix these?

Looks like Pandoc is really fussy about whitespace around the list. I've fixed it up in https://github.com/ros-controls/joss_paper/pull/29

@arfon - The PR has been merged - can you regenerate the paper now?

@arfon - The PR has been merged - can you regenerate the paper now?

Yes, I've regenerated it.

both the version at the DOI and the PDF still look not correct to me. Am I missing something? Maybe I'm getting a cached version?

screen shot 2017-12-05 at 10 45 26 am

screen shot 2017-12-05 at 11 03 05 am

Yep, I think you must be getting a cached versions? See the screenshot of the PDF view and the website.

ok, thanks.

Was this page helpful?
0 / 5 - 0 ratings