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]: eLabFTW - a free and open source electronic lab notebook #146

Closed
whedon opened this Issue Dec 19, 2016 · 40 comments

Comments

Projects
None yet
6 participants
@whedon
Copy link
Collaborator

whedon commented Dec 19, 2016

Submitting author: @NicolasCARPi (Nicolas CARPI)
Repository: https://github.com/elabftw/elabftw
Version: 1.5.7
Editor: @cMadan
Reviewer: @JohnGriffiths
Archive: 10.6084/m9.figshare.4868882

Status

status

Status badge code:

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

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 (1.4.0)?
  • Authorship: Has the submitting author (@NicolasCARPi) 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

whedon commented Dec 19, 2016

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @raamana 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
@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Dec 19, 2016

@raamana - I think you've reviewed for JOSS before, so you know how this works. Let me know if you have any questions for me, otherwise just check things off as appropriate and make comments to the authors here or as issues in the project's repo as makes sense :).

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 19, 2016

Hey, @NicolasCARPi, you need to add a plain text LICENSE containing ones of the OSI approved licenses: https://opensource.org/licenses/alphabetical

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 19, 2016

hey, @cMadan, sure, I will. Looks like it is all self-explanatory.

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 19, 2016

To authors: I was able to follow all the steps outlined here at https://elabftw.readthedocs.io/en/latest/install-mac.html

and landed into an error at the final step.

See the image here:
screen shot 2016-12-18 at 10 57 36 pm

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 19, 2016

The description of the repo could be improved by opening with the 3 questions listed under the GIF and then proceeding with what is currently the first para.

Under the Installation section, authors could summarize the major requirements, write a line or two about the process, rather than simply linking to another site. This helps the user get an overall idea of the level of involvement and effort needed before getting into details.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Dec 19, 2016

Dear @raamana , thank you for your constructive criticism :).

  1. The licence file was removed from the root to be integrated first in a doc/ folder, which was then moved to another repo. I realize now that this repo does not have a plaintext licence file (although it is in the documentation. Let me fix that (it is now done on hypernext branch, I'll merge it to master later, if needed).

  2. Indeed since the 1.4.0 release a few days ago, composer needs to be installed to populate the vendor/ directory. I updated the documentation for Mac. Commit here : elabftw/elabdoc@45e5e3e
    Doc here : https://elabftw.readthedocs.io/en/latest/install-mac.html

  3. The 3 questions have been moved as suggested.

A line or two have been added under the installation section to describe better how it works and what it needs. New readme visible here.

Cheers,
~Nico

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 21, 2016

Hi Nicolas,

The installation instructions for Composer do not work as noted on their website: https://getcomposer.org/download/
screen shot 2016-12-20 at 9 21 57 pm

Perhaps it may be better package it with your codebase to reduce the burden on the user, if at all it is possible?

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Dec 21, 2016

Hello,

You are on a Mac right ? Try with http://.
See this issue composer/composer#5685.

Also, composer should not be packaged with the software, it isn't a correct deployment strategy to do that. I used to have the vendor directory tracked by git, but I removed it to reduce the size of the repo (because all the dev dependencies were coming with it).

Now you should know that the default and recommended way to install this software is with Docker, and the docker image takes care of everything, to highly "reduce the burden on the user", in your terms :). Installing it on a server is very quick and easy, as you can see on the install page.

eLabFTW is a server software, and should be installed on a server. Because of its flexibility it is also possible to install it on a personnal computer (Mac or Windows), but it is not the philosophy.

This software aims for correct and secure cryptography, and this can rise issues on the Mac OS X platform because of its outdated OpenSSL, PHP, versions and libraries. This is why the recommended operating system is GNU/Linux (and inside a Docker container).

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Dec 22, 2016

That clarification helps. I tried to follow the instructions as I was led. What this means is that the user at the very first entry point was not led to the recommended way to install it. So I would recommend updating the instructions to install to follow your line of thought (what you recommend and why), and only unveil more progressively advanced options only as needed, and with appropriate advice.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Dec 22, 2016

Well, there is a WARNING saying exactly that on top of the Mac and Windows install pages. It is also repeated several times that it needs to be installed on a server, in the Readme, on the documentation home page, on the main website. I don't see how I can make it clearer!

a

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Jan 8, 2017

@raamana - are you still able to review this submission?

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Jan 9, 2017

Thanks for the reminder, Chris. I will try to give it another to run and test the software on my Mac.

As @NicolasCARPi points out, it should be installed on a server, and ideally be tested by a team, neither of which I don't have access to, for this purpose. So wondering what would be an ideal way to proceed from here.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Jan 10, 2017

@raamana I can set up a server for you if you wish. It only costs a few dollars per month :) Tell me and I'll give you an IP adress so you can follow this tutorial:
https://www.youtube.com/watch?v=K8CbQl7H5WA

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Jan 17, 2017

Thanks for the offer. However don't you need few people and/or accounts to test the full functionality?

I am quite busy with few major deadlines end of this month. Can this wait until then? Or if another reviewer could be found to assist with this review, that will be great to keep this going.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Jan 17, 2017

@raamana, feel free to register several users.
@cMadan can you put another reviewer in there ? :)

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Jan 20, 2017

@NicolasCARPi - sure, I'll try and recruit another reviewer

@JohnGriffiths

This comment has been minimized.

Copy link
Collaborator

JohnGriffiths commented Jan 28, 2017

@cMadan - I am on board and ready to start reviewing. But despite a lot of browsing this repo it's still not clear to me how I am supposed to do this. Sorry. Please advise.

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Jan 30, 2017

@JohnGriffiths - For the most part you need to see if the statements above in the first post are true and check things off accordingly. If something isn't the case, or you otherwise have a suggestion, either post a comment to @NicolasCARPi here (or ask me) or make an issue in the project's repository (https://github.com/elabftw/elabftw) and we'll sort things out until the submission meets JOSS' guidelines. The guidelines are explained in a bit more detail here: http://joss.theoj.org/about#reviewer_guidelines.

@raamana - the current JOSS submission system only allows one reviewer to be assigned at a time, so I'm going to change it to @JohnGriffiths. Nonetheless, please feel free to continue to open issues in the project's repository and make comments here.

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Jan 30, 2017

@whedon assign @JohnGriffiths as reviewer

@whedon

This comment has been minimized.

Copy link
Collaborator

whedon commented Jan 30, 2017

OK, the reviewer is @JohnGriffiths

@raamana

This comment has been minimized.

Copy link
Collaborator

raamana commented Jan 30, 2017

No problem, I would have more time starting next week, whence I can help with others if you need me to.

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Feb 6, 2017

@JohnGriffiths - do you have any questions about the process that I might be able to help with?

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Feb 27, 2017

@raamana, @JohnGriffiths - are either of you able to review this submission?

@JohnGriffiths

This comment has been minimized.

Copy link
Collaborator

JohnGriffiths commented Feb 27, 2017

Trying to but so far haven't been able to install

elabftw/elabftw#349

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Feb 27, 2017

@JohnGriffiths - thanks for the update!

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Apr 5, 2017

@JohnGriffiths Now that you have access to your own install, did you manage to progress on the review?

Cheers,
~Nico

@JohnGriffiths

This comment has been minimized.

Copy link
Collaborator

JohnGriffiths commented Apr 5, 2017

@cMadan I've been through the cloud instance and the functionality all seems fine. I've updated all the tick boxes above. The only thing I can't give a tick for is installation, since as you know I wasn't able to install successfully on linux.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Apr 5, 2017

That's really too bad that you didn't manage to install. I'll just leave this here:

2017-04-05-231703_789x169_scrot

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Apr 12, 2017

@JohnGriffiths - thanks for your work in reviewing this! I know you spent a good deal of time trying to evaluate it and I really appreciate it

@NicolasCARPi - I am sufficiently satisfied that others have been able to install eLabFTW, and your Github issues are quite active, that I believe that others will be able to set-up the software. If you can provide the DOI to an archive of the current version of the software, I am happy to accept your paper at JOSS.

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Apr 12, 2017

@cMadan Great news :) What do you mean by "provide a DOI to an archive"?

This is a link to the latest version:
https://github.com/elabftw/elabftw/archive/1.5.6.tar.gz

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Apr 12, 2017

@NicolasCARPi - Basically, we require that the archive be stored on a service such as Zenodo or figshare, please see the end of the author guidelines section here (http://joss.theoj.org/about#author_guidelines)

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Apr 12, 2017

@cMadan Thanks for the clarification. Here is the DOI: 10.6084/m9.figshare.4868882 :)

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Apr 13, 2017

@whedon set 10.6084/m9.figshare.4868882 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator

whedon commented Apr 13, 2017

OK. 10.6084/m9.figshare.4868882 is the archive.

@cMadan

This comment has been minimized.

Copy link
Member

cMadan commented Apr 13, 2017

@NicolasCARPi - Perfect, thank you!

@arfon - We're all set to accept! :)

@arfon

This comment has been minimized.

Copy link
Member

arfon commented Apr 14, 2017

@NicolasCARPi - please could you remove the "Links" section as this breaks the paper compilation with pandoc.

# Links

- [Official website](https://www.elabftw.net)
- [Documentation](https://elabftw.readthedocs.io)
- [Live demo](https://demo.elabftw.net)
- [GitHub repository](https://github.com/elabftw/elabftw)

We already link to the GitHub repository in the compiled PDF so this one can be removed and the other links should be cited in the text in the main Summary section (and then the references in a bibtex file). You can see a recent example of how to do this here: https://github.com/ropensci/getCRUCLdata/tree/master/inst/paper

NicolasCARPi added a commit to elabftw/elabftw that referenced this issue Apr 14, 2017

@NicolasCARPi

This comment has been minimized.

Copy link

NicolasCARPi commented Apr 14, 2017

@arfon I made the requested changes (see commit above). Cheers. Nico.

@arfon arfon added the accepted label Apr 14, 2017

@arfon

This comment has been minimized.

Copy link
Member

arfon commented Apr 14, 2017

@JohnGriffiths many thanks for the review here and @cMadan for editing this one

@NicolasCARPi - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00146 ⚡️ 🚀 💥

@arfon arfon closed this Apr 14, 2017

@JohnGriffiths

This comment has been minimized.

Copy link
Collaborator

JohnGriffiths commented Apr 14, 2017

You're welcome. Well done, good work all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment