Closed editorialbot closed 2 years ago
editorialbot
commented
2 years 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 commandsFor example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@editorialbot generate pdfSoftware report:
github.com/AlDanial/cloc v 1.88 T=0.36 s (1302.9 files/s, 103336.0 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Python 296 4543 2890 16674
HTML 125 667 1 4966
PO File 2 756 1208 2236
Markdown 12 427 0 1231
JSON 8 0 0 664
JavaScript 12 55 7 479
CSS 1 25 3 161
YAML 2 5 3 140
make 1 14 0 47
Bourne Shell 2 12 5 32
Dockerfile 1 11 0 25
SVG 6 0 0 22
INI 1 3 0 19
TeX 1 1 0 16
PHP 1 0 0 9
-------------------------------------------------------------------------------
SUM: 471 6519 4117 26721
-------------------------------------------------------------------------------
gitinspector failed to run statistical information for the repositoryReference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- None
MISSING DOIs
- 10.17487/rfc6238 may be a valid DOI for title: TOTP: Time-Based One-Time Password Algorithm
INVALID DOIs
- NoneWordcount for paper.md is 974
editorialbot
commented
2 years ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
samhforbes
commented
2 years ago 
htwangtw
commented
2 years ago 
oliviaguest
commented
2 years ago @jagnobli can you check out that missing DOI when you get a sec — no big rush?
oliviaguest
commented
2 years ago @editorialbot generate pdf
editorialbot
commented
2 years ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
oliviaguest
commented
2 years ago 👋 @samhforbes, @htwangtw: thank you both so much for agreeing to review this! ☺️
Any code-related questions, feel free to open issues on the package's repo itself and then link to whatever issue you open from here. This means we can all keep track of what is being asked of the authors from this thread. Everything important about the PDF or the package, like high-level questions to me or @jagnobli, discussions and feedback, just leave it here as a comment directly. ✨ 🌷
samhforbes
commented
2 years ago @jagnobli, this looks exciting. As far as I can see we would need a mpib git account to log in and create issues, so I currently can't raise issues on the repository. Is this the case or have I missed something (the latter is very possible...)
jagnobli
commented
2 years ago @jagnobli can you check out that missing DOI when you get a sec — no big rush?
sure thing, @oliviaguest will do that next week...
jagnobli
commented
2 years ago @jagnobli, this looks exciting. As far as I can see we would need a mpib git account to log in and create issues, so I currently can't raise issues on the repository. Is this the case or have I missed something (the latter is very possible...)
@samhforbes I wasn't aware that issues should be opened over there - this is a bit annoying, sorry. Will ask a colleague to set up a guest account for you (and @htwangtw as well). Could you please pm me your mail address that should be used as account?
oliviaguest
commented
2 years ago @jagnobli, this looks exciting. As far as I can see we would need a mpib git account to log in and create issues, so I currently can't raise issues on the repository. Is this the case or have I missed something (the latter is very possible...)
Ah, true. Sorry and thanks! I actually noticed that and then forgot. @openjournals/joss-eics (see above comment by @samhforbes) shall we just post here in this thread — thoughts? Would that work @jagnobli event tho it makes things harder (probably) to keep track for you?
danielskatz
commented
2 years ago This isn't just a problem for the reviewer, but it's a problem for meeting the 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 criterion as well. In some cases, an option has been to create a github mirror of the software for the purpose of issues and contributions, but it's up the author to figure how the software will meet this criterion
oliviaguest
commented
2 years ago @jagnobli, this looks exciting. As far as I can see we would need a mpib git account to log in and create issues, so I currently can't raise issues on the repository. Is this the case or have I missed something (the latter is very possible...)
@samhforbes I wasn't aware that issues should be opened over there - this is a bit annoying, sorry. Will ask a colleague to set up a guest account for you (and @htwangtw as well). Could you please pm me your mail address that should be used as account?
Thanks, and it would likely be only a step 0. This is because given @danielskatz comment above, your code currently does not indeed meet our guidelines.
jagnobli
commented
2 years ago hmmm, perhaps I am a little small-minded here (sorry):
I actually think that we provide clear "guidelines for third parties wishing to [...] Contribute [...] Report issues [...] Seek support" in a contributing markdown of our repo.
So, from my point of view the community guidelines should be more explicit if our approach is insufficient...
What do you think?
oliviaguest
commented
2 years ago @jagnobli this is a bit errant, so I will discuss with other @openjournals/joss-eics. For the time being, can you allow @samhforbes and @htwangtw access to the repo, please?
jagnobli
commented
2 years ago should now present the missing doi:
@editorialbot generate pdf
oliviaguest
commented
2 years ago @editorialbot generate pdf
oliviaguest
commented
2 years ago @jagnobli @editorialbot command has to be a comment on its own to work 😊
editorialbot
commented
2 years ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
oliviaguest
commented
2 years ago @editorialbot check references
editorialbot
commented
2 years ago Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.17487/rfc6238 is OK
MISSING DOIs
- None
INVALID DOIs
- NoneI see @jagnobli @samhforbes are discussing how to make the documentation more clear for installation, here: https://git.mpib-berlin.mpg.de/castellum/castellum/-/issues/158#note_26799. Might be useful to check out for @htwangtw too.
In general, please link to any issues you all open relating to this, so we can all keep track.
samhforbes
commented
2 years ago Hi @jagnobli I think from the example deployment this looks exciting. I appreciate there are merge requests where extensive additions are being made to docs, however I will comment on the main branch at this stage.
To avoid talking across purposes on the project repo, I'm going to highlight some issues as I see them here, and possibly open isolated issues where appropriate on the project repo. The reason is some of these are more wider scope issues or relate more to the paper than to the software, others may want the watchful and wise input of @oliviaguest
Installation: https://git.mpib-berlin.mpg.de/castellum/castellum/-/issues/158#note_26799. I currently can't verify installation, only an example deployment, which works. Note this works on AMD but not ARM64, and this might be worth documenting?
Documentation is where I have a few things to highlight.
Software paper:
jagnobli
commented
2 years ago Hi @samhforbes,
thanks for your valuable feedback. We will work on your requests. To some of your points I have some direct answers or comprehension questions though:
* Installation instructions.
Not sure if this is helping: We currently see our main target group to be facilities that provide this service to their researchers (rather than having individual researches installing it locally on their machines). Therefore, to us "installation" means "setting up a local test deployment to check if it is worth it" and you were able to set this up...maybe you could elaborate a little more on what's missing from your point of you?
(your technical question will be discussed in the other repo issue board)
* Automated tests: I can see you have tests and CI here
To me this statement lacks some kind of request. Hence, should we do something about this?
* Community guidelines. From what I can see the contributing md document contains this, _however_ the link given to contact takes me to the mpib lifespan psychology homepage, and I can't see clear links to castellum from there, nor the contact details for the authors with any great ease.
We simply forgot to update the link to the new location. This will be fixed soon. Do you think this would be enough to check it in the check list?
Software paper:
- Some discussion of the state of the field, comparison with existing platforms would be good to meet these guidelines.
I am a little unsure here as the original checklist states "packages" (maybe a question for @oliviaguest?): Does this mean we should only present other open source tools that may come with a similar feature set or should we present commercial online services that provide similar feature sets (I would have trouble in finding this helpful as there are some but would be a different scope to review them; to me, it comes down to "make or buy" and it feels somehow strange to put this in a paper about an open source tool?)
samhforbes
commented
2 years ago Hi @jagnobli Excellent stuff.
installation instructions. Yes some of this is at your discretion of course, since the audience is specific, and I can understand that these would be facilities / centres / departments running this, not individuals - and I think you are reasonably clear about this in the docs. However, the local test deployment works, but not on all machines, and the login instructions were not super well documented, so these instructions could be added for the test deployment perhaps? For the actual installation as it is intended my point is more that I can't test this as it stands (of course since I am not the intended installer). So as long as the instructions are adequate for a facility manager to set this up, then that meets remit.
Automated tests. Yup, sorry for the confusion, just working my way through the list and noted your testing seemed good!
Community guidelines. I see this is now updated and links correctly to contact details. Ticked and done.
Software paper. I'm happy for @oliviaguest to advise here as well, but (and you know the state of the field better than me) Castellum seems to be fairly unique in what it provides - if no other platform provides a comprehensive, open source, GDPR compliant system, then I would say this should definitely be a strength of Castellum mentioned in the paper :)
From where I stand, I like the platform and the way it runs, and am impressed with the work done on it by the three authors. I have tested installation and functionality of the example deployment and am satisfied, pending the documentation of installation details and other issues mentioned above and in the linked issue. Obviously I have only tested the example deployment, so some details of a full install I have to take at face value.
jagnobli
commented
2 years ago @editorialbot generate pdf
editorialbot
commented
2 years ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
jagnobli
commented
2 years ago @editorialbot check references
editorialbot
commented
2 years ago Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.3414/ME14-01-0133 is OK
- 10.1007/s40881-015-0004-4 is OK
- 10.17487/rfc6238 is OK
MISSING DOIs
- None
INVALID DOIs
- None@editorialbot generate pdf
editorialbot
commented
2 years ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
jagnobli
commented
2 years ago Paper now contains a section on "State of the field". I decided to only mention open source tools by name. Let me know if you think I should also promote commercial providers ;)
htwangtw
commented
2 years ago Hi @jagnobli!
This is a very cool project and provides a solution to the privacy issue in human research! I cannot fully verify the functionality and I do have a lot of suggestions for the documentation. In the check list, I have ticked off items that pass the OK criteria in the JOSS guideline.
Installation - I would discuss the demo and the developer installation separately. There's already an open issue on this topic.
I would pass these three points purely based on the demo, but I imagine the developer installation is equally important when the software is eventually deployed at different research site. If editor @oliviaguest can chip in on the scope of the review here, that would be fantastic!
The information in each link on the readme file are great, but there's a lot of friction when I am trying navigate the documentations either from an end user's or a dev's perspective. How the information is organised can be improved. I would imagine the product website is the point of entry for your target audience. I cannot get to documentation relevant to a sys admin or a end user easily.
Side note, the Read the Docs layout tricked me to think I could find API documents in the user guide.
This is clear and I see no issue with this section. Excellent work!
jagnobli
commented
2 years ago Hi @htwangtw,
thanks very much for your detailed feedback.
As you can see in the repo we are currently working on improvements on the documentation. We once started with the idea that developers most likely will stay within the repo. So, we simply put all relevant docs for them there (castellum/docs). For end users we set up the external documentation with the read the docs style. Now as you seem to have trouble in seeing/liking this concept: Do you have an idea how to highlight our separated approach more clearly?
I am somehow a little confused by your statement that you can only verify that it is working in demo mode locally. To me, if demo is running locally on a desktop machine it will sure run fine on a dedicated web service environment? So, I also think that @oliviaguest may be able to help us out on what is really needed to verify?
This is a recurring story about the community guidelines, sorry: I understand your concerns about opening issues but I am not really seeing the difference to other git repos? You could always argue that you frequently need an account for github/gitlab/bitbucket/... to open up issues in a git repo? From my point of view, we simply offer a mail contact instead of a register button to create a new account?
htwangtw
commented
2 years ago @jagnobli Thanks for the quick response!
I am following my understanding of the JOSS guideline and consider people who will actually deploy this service as the target audience of the paper. If the intention is to make others to deploy your software, I do think the current organisation of documentation will create friction for adoption.
Also, I feel there's a lot of my own bias, coming from a view point of no front end developing experience and did not look into front end project workflow before reviewing this piece of work.
Do you have an idea how to highlight our separated approach more clearly?
I think as long as the information about deployment is also in the current user doc in a section for administrator, it will be fine. That's the only piece of information that need to be easily findable for externals that's not there. To separate this from the internal dos, some extra instruction on how to configure for their own system, rather than just a demo people can press play with no issues.
If this is resolved I think I am happy to tick off installation.
To me, if demo is running locally on a desktop machine it will sure run fine on a dedicated web service environment?
This is a recurring story about the community guidelines
I am really not sure if the reviewers need to verify the developer installation. Email as the main channel is reasonable if you are expecting frequent code contribution, or just have a different development workflow. For these two comments, I guess it will depends on how to handle third party contribution to your code base, and what's the expectation of JOSS. The current code contribution guideline reads
oliviaguest
commented
2 years ago I suggest @jagnobli addresses the documentation issues raised above as best they can. And I want to repeat that the instructions for "[c]ontribut[ing] to the software" should also be clear (https://joss.readthedocs.io/en/latest/review_criteria.html#community-guidelines). If the reviewers cannot understand how to contribute, that's a sign that these can be improved.
jagnobli
commented
2 years ago Thanks four your suggestions.
I suggest @jagnobli addresses the documentation issues raised above as best they can.
We are on it :)
And I want to repeat that the instructions for "[c]ontribut[ing] to the software" should also be clear (https://joss.readthedocs.io/en/latest/review_criteria.html#community-guidelines). If the reviewers cannot understand how to contribute, that's a sign that these can be improved.
Sorry for being reluctant here: I can not see that the reviewers do not understand how to contribute, right @htwangtw and @samhforbes? To me, it's a general question: Do the JOSS guidelines require git platforms that allow everyone to create a account by themeselves or is it ok to ask potential contributors to contact a maintainer via mail to ask for an account (it's the same outcome from my point of view)?
htwangtw
commented
2 years ago Sorry for being reluctant here: I can not see that the reviewers do not understand how to contribute, right @htwangtw and @samhforbes? To me, it's a general question: Do the JOSS guidelines require git platforms that allow everyone to create a account by themeselves or is it ok to ask potential contributors to contact a maintainer via mail to ask for an account (it's the same outcome from my point of view)?
To be honest the current guideline reads like you are not prepared for third party contribution. For me the problem is not hosting the code on institutional GitLab, it's that the current contribution guideline section doesn't really out line the process described in the discussion here. I would be happy if the current contribution guideline on software says:
Even with the process you described, the instruction should be followable. I have also commented on the installation issue to report the error I encountered and find a potential solution See here.
My only reference point of frontend software was the one PR I contributed to a web app using React framework, and I knew nothing about TypeScript at that point. The said webapp was not expecting third-party contribution regularly. I am not a front-end dev, but I could follow through their instruction and set up a testing environment locally. I don't think improving the documentation on dev environment setup is an unreasonable request.
xi
commented
2 years ago I don't think improving the documentation on dev environment setup is an unreasonable request.
Not at all. All this feedback is really helpful. It is just sometimes hard to understand what exactly needs to be improved. That's why we are asking so many questions.
To be honest the current guideline reads like you are not prepared for third party contribution.
We have the following paragraph in contributing.md:
"You can send small changes in any format you would like. For bigger changes, please contact us so we can give you access to our gitlab server."
What do you think is missing here? Or is it more an issue of structure?
samhforbes
commented
2 years ago Hi there, I don't want to speak for @htwangtw at all but one issue as I see it is the structure: You have contributing guidelines for the development. I'd like if there was a link to it from the README, because it needs to be easier to find, but that may just be me. However, you have end user documentation separately, which is OK if you want to choose that structure, at least from my perspective, but there are no paths to contact or raise issues for end users on the documentation that is currently provided. So simply putting this information for end users would be a start. I suspect that as your user base grows (and I hope it does), end users will be a decent proportion of people with questions / issues.
I'm just putting myself in the shoes of a user who runs into something. If I'm googling there's a few different sites I can go to, but I need to know clearly where I go and what I do to contribute / ask questions.
From my personal point of view as long as all this is clearly stated, that's fine. If you end up getting a lot of emails about certain aspects, that's where public issues might be an easier way to manage it, perhaps? But that's just my take.
oliviaguest
commented
2 years ago To me, it's a general question: Do the JOSS guidelines require git platforms that allow everyone to create a account by themeselves or is it ok to ask potential contributors to contact a maintainer via mail to ask for an account (it's the same outcome from my point of view)?
@jagnobli I don't want to decide without input from @openjournals/joss-eics and they (or many) are on holiday. Is it OK to try your best for the other issues that are about how and what you're saying in the documentation until they/we come back from vacation? 😊
danielskatz
commented
2 years ago Typically, JOSS-reviewed software is on a platform where anyone can open an issue or create a PR without having to get permission from the code author. (I'm writing as an AEiC)
jagnobli
commented
2 years ago Thanks for all your valuable input. However, I feel like I'm stuck here. Therefore, I try to sum up my understanding of this discussion and will ask for specific feedback at the end.
GitHub, GitLab.com or Bitbucket as we can't force employees who are currently participating "within their working contracts" to create (or use private) accounts over there. In addition, we do not see benefits in mirroring the repo to allow opening issues. As this is a small core team we would not be able to manage issues a two locations, instead we would have to set up email notifications for the mirror. Hence, from my point of view this would add project complexity but result in the same work flow as the suggestion in our current contributing guideline that clearly states please contact us via mail. Also, I like to mention that our institute is not able to open up its GitLab to create accounts on demand via button click but it's not the code owner who is deciding on the creation of a guest account, so the implication that there might be an unfair selection of contributors appears groundless to me.Specific/explicit feedback needed:
Thanks again (and we understand that it's vacation season. So, we are not in a hurry...)
oliviaguest
commented
2 years ago @jagnobli @xi thanks for taking the time to try and digest what is being said. I suggest, for now, to apply the paused label to allow you to apply the changes to the documentation and anything (other than the community contributions issues) that the reviewers have signposted without any further (required) back and forth. Please feel free to take your time and, if you can, only comment back here when you need us — I hope this releases some pressure, and I am excited to see your edits in due course. ☺️
For the time being, @samhforbes and @htwangtw, this submission will be considered paused, but of course you may provide feedback as needed if there are any changes over and above what you have already outlined. Please do not, until I have messaged you here, feel the need to keep checking this for now. And thank you already for the time you have generously spent on this.
jagnobli
commented
2 years ago @oliviaguest As described over there we made some substantial changes to the docs. We hope that this may allow us to reopen the review process?
oliviaguest
commented
2 years ago @oliviaguest As described over there we made some substantial changes to the docs. We hope that this may allow us to reopen the review process?
Happy to unpause. Please post a list of changes here (feel free if already created, to copy-paste) to allow them to be more easily discussed by us all in case there's more to say or new things come up. Thanks. 😊
Submitting author: !--author-handle-->@jagnobli<!--end-author-handle-- (Timo Göttel) Repository: https://git.mpib-berlin.mpg.de/castellum/castellum Branch with paper.md (empty if default branch): joss-submission Version: v0.82.0 Editor: !--editor-->@oliviaguest<!--end-editor-- Reviewers: @samhforbes, @htwangtw Archive: 10.5281/zenodo.7177876
Status
Status badge code:
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
@samhforbes & @htwangtw, 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:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @oliviaguest 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 @samhforbes
📝 Checklist for @htwangtw