openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: CRSocket: A web app component facilitating the administration of digital trials from a separate device #5658

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@henrikdvn<!--end-author-handle-- (Henrik Dvergsdal) Repository: https://github.com/henrikdvn/CRSocket Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@britta-wstnr<!--end-editor-- Reviewers: @rabdill, @lukaszjablonski Archive: 10.6084/m9.figshare.24009225.v3

Status

status

Status badge code:

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

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

@rabdill & @lukaszjablonski, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @britta-wstnr know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

πŸ“ Checklist for @lukaszjablonski

πŸ“ Checklist for @rabdill

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.05 s (1427.8 files/s, 197578.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JavaScript                      13            819            748           3059
HTML                            28            229              0           2194
PHP                             20            276            284            852
Markdown                         3            143              0            619
TeX                              1              9              0            152
CSS                              3             50              4             87
YAML                             1              1              4             18
-------------------------------------------------------------------------------
SUM:                            69           1527           1040           6981
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.neuroimage.2022.119241 is OK
- 10.1371/journal.pone.0260695 is OK
- 10.1111/desc.13193 is OK
- 10.1016/j.infbeh.2021.101649 is OK
- 10.1080/15248372.2015.1061528 is OK
- 10.3389/fpsyg.2016.01021 is OK
- 10.1002/acp.3569 is OK
- 10.1177/0894439319851503 is OK
- 10.3233/JAD-160545 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

Wordcount for paper.md is 1079

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

britta-wstnr commented 1 year ago

Hello again! πŸ‘‹β€¨ @rabdill @lukaszjablonski FYI @henrikdvn

This is the review thread for the paper. All of our higher-level communications will happen here from now on, review comments and discussion can happen in the repository of the project (details below).

πŸ““ Please read the "Reviewer instructions & questions" in the comment from our editorialbot (above).

βœ… All reviewers get their own checklist with the JOSS requirements - you generate them as per the details in the editorialbot comment. As you go over the submission, please check any items that you feel have been satisfied.

πŸ’» The JOSS review is different from most other journals: The reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention the link to https://github.com/openjournals/joss-reviews/issues/5658 so that a link is created to this thread. That will also help me to keep track!

❓ Please also feel free to comment and ask questions on this thread if you are unsure about something!

🎯 We aim for the review process to be completed within about 4-6 weeks* but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

*I do appreciate of course that it is summer vacation time πŸ–οΈ - also a heads-up that I will have limited availability myself for the next two weeks.

lukaszjablonski commented 1 year ago

Review checklist for @lukaszjablonski

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rabdill commented 1 year ago

Review checklist for @rabdill

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rabdill commented 1 year ago

The short version: This application uses PHP 7.4, which is no longer supported. Is that an issue, @britta-wstnr?

The longer version: This is interesting work and I'm making my way through the review checklist, but I wanted to raise a question about the use of PHP: The documentation currently states that CRSocket can run on "any web server that supports PHP 7.4 and common Apache HTTP functionality... Migration to PHP 8 will require some minor adjustments."

PHP 7.4 left active support in 2021, and, as of last November, is no longer receiving even critical security patches. I don't see any journal policy about this, but I'm unsure of whether it's a good idea to publish a software paper that relies on a platform that is explicitly no longer supported, particularly when it is intended to be the foundation of larger applications that 1) will probably take a little while to develop, and 2) will probably be used for months or years at a time. So it's a little outdated right now, but it will be considerably farther removed by the time someone integrates CRSocket into their application and deploys it for a study.

On the other hand, most online PHP applications are still using PHP 7.4, so this is a common issueβ€”indeed, using a secure and supported version of PHP seems like the more unusual situation. So sure, it isn't best practice, but it's not rare, and there's no requirement that CRSocket applications actually communicate over the internet instead of a local network.

Looking at past reviews, I can't find many examples of situations like this: A 2020 review noted a similarly unsupported version of PHP, but that wasn't the deciding factor in its eventual rejection. A reviewer noted that a 2017 submission was using Python 2.7; the submission was accepted, but Python 2 was not yet end-of-life. This was also raised in another 2019 review, but Python 2 was not EOL then either, and the issues raised were unrelated.

Anyway, I'm sorry if this is the wrong format for a question like this, but I didn't want to open an issue in the submitting repository for updating PHP if this was just an editorial call that I'm not supposed to be involved in.

henrikdvn commented 1 year ago

Regarding the PHP version: There was an issue with version 8.0 at some point, but it now seems to work well with 8.2.5 which is the latest supported by my ISP. Haven't tested extensively, and I don't know about all versions in between, but I have now taken the chance to state that it supports "7.4 or later".

britta-wstnr commented 1 year ago

Hi @radbill,

thanks for raising this - and thanks for looking into comparable cases as well. Indeed, JOSS does not have a requirement for this. I raised this with the EiC team, and there the major point was about whether the software can be easily installed and run. Looking at those points and also at @henrikdvn's answer, what do you think about it now, @rabdill ?

Thanks for your thorough work! Britta

lukaszjablonski commented 1 year ago

I think it's OK if it is not tested on all available PHP versions and if it works with still supported 8.2.5 it should be fine.

@henrikdvn, maybe you could add note in the documentation on which versions of PHP it was tested?

henrikdvn commented 1 year ago

@henrikdvn, maybe you could add note in the documentation on which versions of PHP it was tested?

Done

henrikdvn commented 1 year ago

@rabdill, @lukaszjablonski Hopefully, I have now addressed all issues mentioned above

lukaszjablonski commented 1 year ago

@henrikdvn: Just not to open an issue for something that small, there is a typo in CONTRIBUTING.md: is "respoind", should be "respond".

henrikdvn commented 1 year ago

"respoind", should be "respond".

Fixed :)

henrikdvn commented 1 year ago

Issue #3 fixed

lukaszjablonski commented 1 year ago

@henrikdvn: In paper.md please change "eg." to "e.g." and "well defined set" to "well-defined set".

lukaszjablonski commented 1 year ago

@henrikdvn: In paper.md I would also recommend checking consistency between terms in main text and figure: "CRSocket" vs "crSocket ", "CREvent" vs "crEvent", "clientid" vs "clientId". One could also try to use for those terms inline code quotes markup in main text but I don't know what is JOSS style guide saying here (@britta-wstnr, would ytou know?).

henrikdvn commented 1 year ago

Have fixed spelling mistakes, added a figure caption and included a section on basic naming conventions in the paper and on the Terminology page.

henrikdvn commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

lukaszjablonski commented 1 year ago

For now, I have only two more points on my checklist to complete:

  1. In a section "Documentation" - "A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?" - I find such statement in the README.md file of the repository. However, in documentation referenced in same README.md (https://henrikdvn.github.io/CRSocket/ or https://neurolab.no/docs/crsocket/) there is no such statement. @britta-wstnr, what would be a take o JOSS? If that is OK with JOSS I'll be fine to check it out in my checklist.
  2. In section "Software paper" - "State of the field: Do the authors describe how this software compares to other commonly-used packages?" -Although revision to the paper was made, I think that is still missing in the paper. I dropped more extensive comment on that in already closed issue (henrikdvn/CRSocket#2).
henrikdvn commented 1 year ago

Have included a short introduction of functionality and purpose in the documentation index page. Will try to address the second point tomorrow :)

henrikdvn commented 1 year ago

Have added content related to currently available software, and software origin in the "Statement of need" and "Architecture" sections respectively.

henrikdvn commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

britta-wstnr commented 1 year ago

Awesome progress here @lukaszjablonski and @henrikdvn ! 🌱 Regarding your questions, @lukaszjablonski:

  1. Statement of need: It looks like @henrikdvn has solved this by including text in the documentation pages? Can you confirm this addresses the point you were raising @lukaszjablonski ?
  2. Style guide: I can see @henrikdvn added a section to the paper detailing naming conventions for the project. While not explicitly stated in the JOSS documentation, we do recommend using backticks for variable names or package names etc., see our example paper: https://joss.readthedocs.io/en/latest/submitting.html#example-paper-and-bibliography
henrikdvn commented 1 year ago

Have added backticks to all code-related names in the paper.

lukaszjablonski commented 1 year ago

From my side all points are checked and the paper is ready for the publication in JOSS.

In general, the author presents a well-documented working implementation of the Controller-Responder Socket (CRSocket). This implementation allows for creation of digital tasks where experimenter (in the paper called "testers") using a controller device administer trials on a separate device operated by experiment subject (in the paper called "responder"). As an example of CRSocket use, the author refers to Early Childhood Inhibitory Touch Task (ECITT) which is used in experimental psychology but its easy adaptation allows for creation of different tasks (controller/responder web app pairs).

CRSocket runs on a web server (online or local) that does not require a lot of resources. CRSocket client as a system-independent web-based application can be run on a variety of devices that run modern web browsers so that affordable devices could be employed. In my opinion, that makes CRSocket attractive for the number of users looking for tool that could implement tablet-based tasks.

lukaszjablonski commented 1 year ago

@britta-wstnr, of course that is my recommendation and the decision has to be made by you and JOSS team. I guess we still need to hear the opinion of @rabdill too.

lukaszjablonski commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

henrikdvn commented 1 year ago

So, @rabdill, what's your take on this paper?

rabdill commented 1 year ago

Hi @henrikdvn , I'm sorry for the delay. I will be able to complete my review early this week

britta-wstnr commented 1 year ago

Thanks @rabdill! πŸ™

britta-wstnr commented 1 year ago

@lukaszjablonski Thank you very much for your thorough and detailed review, I appreciate it!

rabdill commented 1 year ago

Hi all, I'm grateful for your patience. I'm also delighted to say my checklist is complete, @henrikdvn did a great job responding to our feedback, and I don't have any objections to publication in JOSS. Please let me know if there's anything else I can do.

britta-wstnr commented 1 year ago

@rabdill Thank you very much for your review and the thorough look you had at the work! I appreciate it! πŸ™

henrikdvn commented 1 year ago

Thank you all. @rabdill, @lukaszjablonski and @britta-wstnr. Really appreciate your work. I like the dialogue-oriented approach of this journal and hope to contribute in the same way at some point, once I have learned how it works.

britta-wstnr commented 1 year ago

@henrikdvn - thanks! You can always sign yourself up as a reviewer here: https://reviewers.joss.theoj.org/join πŸ™‚

I will now have to do some checks as the editor - I will keep you posted @henrikdvn, there will be a few things to do for you - I will let you know!

britta-wstnr commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

britta-wstnr commented 1 year ago

@rabdill I just see you have not checked all the boxes in your checklist - would you mind double-checking that and taking care of it? Thanks a lot! πŸ™

britta-wstnr commented 1 year ago

and, for reference/bookkeeping since not all was tagged here, the issues related to this review: https://github.com/henrikdvn/CRSocket/issues/1 https://github.com/henrikdvn/CRSocket/issues/2 https://github.com/henrikdvn/CRSocket/issues/3

britta-wstnr commented 1 year ago

Hi @henrikdvn - we can now move onto the next steps (creating an archive etc.) ✨ The steps for you are listed in the checklist above. Relevant for you is the section "Additional Author Tasks After Review is Complete".

Could you take care of these points, post the output that is asked (DOIs) for here, and confirm once you went through all of them? Then I can move on to the other steps. Thanks! πŸ™

henrikdvn commented 1 year ago

@britta-wstnr Wasn't able to tick items on the list, but I have now:

Double-checked author info Released v1.0.0 Archived the repository on figshare , doi: 10.6084/m9.figshare.24009225 Checked title, author info and license on figshare

rabdill commented 1 year ago

@rabdill I just see you have not checked all the boxes in your checklist - would you mind double-checking that and taking care of it? Thanks a lot! πŸ™

Sorry I missed this @britta-wstnr. From my side, it looks like all the boxes are checked, and the final edit, from last week, shows the same. Just wanted to confirm it looked correct now.

britta-wstnr commented 1 year ago

@rabdill - oh, I thought you changed it in the meanwhile as indeed they all were showing up as checked when I circled back yesterday. Must have been a glitch then - sorry for the confusion!

britta-wstnr commented 1 year ago

@henrikdvn - thanks! I will take care of it soon!

britta-wstnr commented 1 year ago

@editorialbot check references