editorialbot commented 1 year ago

Submitting author: !--author-handle-->@wergo@faroit<!--end-editor-- Reviewers: @stefan-balke, @rlskoeser Archive: 10.5281/zenodo.11262560

Status

Status badge code:

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

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

@stefan-balke & @rlskoeser, 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 @faroit 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 @stefan-balke

📝 Checklist for @rlskoeser

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.80 s (121.5 files/s, 291791.9 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JavaScript                      46           8272          11594         194080
XML                              2              0              0           7561
SVG                             20              5              9           6325
CSS                             13            379            135           2013
HTML                             2             31             40           1411
Markdown                         7             78              0            514
TeX                              1             20              0            194
JSON                             2             11              0            182
Python                           4             16              1             77
-------------------------------------------------------------------------------
SUM:                            97           8812          11779         212357
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 1 year ago

Wordcount for paper.md is 1276

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.17613/fc1c-mx52 is OK
- 10.18061/emr.v16i1.7643 is OK
- 10.14361/9783839451458-015 is OK
- 10.1145/3543882.3543892 is OK
- 10.1525/JAMS.2016.69.1.273 is OK
- 10.1145/3243907.3243911 is OK
- 10.1007/s00799-018-0262-x is OK
- 10.1145/3543882.3543891 is OK
- 10.1145/2872518.2890529 is OK
- 10.18716/ride.a.15.2 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1145/3358664.3358666 is INVALID because of 'https://doi.org/' prefix

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

faroit commented 1 year ago

@wergo happy to announce that we found two reviewers so quickly. While the reviewers are getting started, maybe you can already go ahead and try to fix the invalid DOI error?

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:

stefan-balke commented 1 year ago

Review checklist for @stefan-balke

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/mei-friend/mei-friend?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@wergo) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

rlskoeser commented 1 year ago

Review checklist for @rlskoeser

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/mei-friend/mei-friend?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@wergo) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

wergo commented 1 year ago

@wergo happy to announce that we found two reviewers so quickly. While the reviewers are getting started, maybe you can already go ahead and try to fix the invalid DOI error?

Thanks for reviewing!

DOI error is fixed (superfluous https://doi.org/ removed).

stefan-balke commented 1 year ago

Hi @wergo,

thanks for your contribution. I think the mei-friend is a very valuable contribution to the field. The contribution as well as the paper are in very good shape. However, the documentation and the architecture of the actual implementation could be enhanced.

I would propose the following additions:

Add the instructions for developers here: https://mei-friend.github.io/docs/advanced/dev/ There are some in the INSTALL.md but they are mentioned nowhere and I found them by chance. Also: Is it really necessary to have a GitHub App registered for developing the core JS functionalities? Maybe there could be different levels of documentation (running locally, running locally + development, optional: Deploying on your own lab server --> links to other tutorials etc.)
The architecture of the app itself is a client-server app using flask. However, the code itself is not split into client and server components. Could you lay out the architecture and the tasks of the client and the server respectively?
The developer documentation is missing completely. I know, this is much work but will increase the value of the software tremendously. Start with a documentation of the folder structure and the files. Give pointers to the main files and where developers could find a handle. It could also be a short scree-cast showing people how to implement new features.
Unit-Tests are missing. Start with testing core functionalities such as importing MusicXML files and checking against an expected MEI-output. These also give devs a good starting point to understand the inner functionalities of your software.
The manual download of codemirror5 should be automated, e.g. with a git submodule.

Keep up the great work. Since JOSS is focused on the Software aspect, please include the above mentioned, mainly docs, issues.

stefan-balke commented 1 year ago

As requested by the editor, I added issues to the software's repo.

wergo commented 1 year ago

Dear @stefan-balke, many thanks for the valuable comments and suggestions which we will be carefully including into our submission.

faroit commented 1 year ago

@wergo to keep this review issue clean, please discuss unit test alternative in the respective issue

rlskoeser commented 1 year ago

Hi, @wergo, some feedback for you on the software paper. Your statement of need is clear, but I think a couple of aspects could be stated more explicitly:

What is the intended audience? Is it primarily/only for scholarly use, and could you clarify that more, e.g. does that include research and education? You mention it in the intro that it provides a friendly, interactive interface - do you hope it will be useful for a more general public, or is MEI to technical/specialized for that? You also mention uptake by the MEI community; does that overlap exactly with your target audience or do you hope it will have wider appeal?
Please clarify the relationship to other available packages and solutions. You talk about MEI in comparison to MusicXML and other formats, but need to pay equal attention to your tool. You mention commercial notation software in passing, but don't name any - that may be fine, I suppose there are a lot of them and they serve a somewhat different purpose, but it could help to differentiate the goals a little more clearly. Am I understanding correctly that mei-friend is the only MEI editor of this type? If you so, you should definitely say that!

rlskoeser commented 1 year ago

I've now worked through the installation instructions and was able to get mei-friend working locally - very cool to see the musical notation and the xml linked and editable. This seems like a really powerful and elegant interface, and I found myself wishing there was a tutorial to give me a little more of an orientation to the kinds of things I might be able to do, but I was able to poke around and try a few things out. When you update the installation documentation, you should make sure you link to the documentation you have on github pages about how to use it. A tutorial is a nice-to-have, not essential - you have thorough usage documentation already.

installation

My feedback on the installation has significant overlap with what Stefan has already pointed out.

Installation instructions should be easier to find; there should be a link in the readme. I don't feel strongly about whether it should be a markdown file or a page on your help pages, but it should be findable from both.
I skipped the GitHub OAuth setup because I wanted to see what would happen. I didn't get any errors, but I don't think I've tried any functionality yet that requires it. It would be helpful to make it clearer what this is needed for and where it comes into play.
It took me a couple of tries to get the codemirror download installed and put in the right place (at first I nested it incorrectly because you have an existing CodeMirror/ directory already in the application). I'm not certain which codemirror components you're using (due to the code organization concerns that Stefan already raised), but it looks like you should be able to install it from npm; if so, that would be preferable to a git submodule.
Consider making the application installable as a python package, at least from github even if you don't want to publish it on pypi. You can do this by adding a pyproject.toml file that describes your package, and includes the python requirements (this replaces requirements.txt). You will need to do configure the package to make sure the necessary non-python static files are included, and I think an npm install would still be needed for the js dependencies.
I noticed you have a package.json file under an npm directory, but I'm not sure if you're using it; I see you also have a bunch of js files and licenses in a deps module. I recommend installing these dependencies from npm where possible.
Your installation documentation includes an outdated reference bout setting the js mimetype, it looks like you've already added that.
For the production deployment, I suggest linking to the default Flask deploy instructions and then adding any adjustments or configurations that are specific to your application. You could do something similar for the python virtualenv instructions.

I second Stefan's comments about the code organization, developer documentation, and tests.

community guidelines

I'm glad to see you're seeing issue templates, that's great. You might think about revamping your public repertoire submission template so that it looks more like a form with sections, and documentation with each section instead of all at the top. (Basically, structure them a bit more like the bug and feature request templates).

GitHub has documentation and examples for making a contributing guidelines document: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors

faroit commented 1 year ago

@rlskoeser Thanks for your in depth review.

Can you please open issues in the mei-friend repository and link back to this issue here so that we can better track down the progress. As I understand, some of your review points are just suggestions, so feel free to only convert "mandatory" points issues. 🙏

rlskoeser commented 1 year ago

@faroit thanks for the reminder, I'll create issues to document these (at least the essentials) when I'm able to do so.

@faroit question for you: how much of the functionality should I test? Should I try everything covered in the readme? I think the major points I haven't tried are the portions using github auth and annotation.

rlskoeser commented 1 year ago

I've created issues on mei-friend based on my review.

wergo commented 1 year ago

Dear @rlskoeser thank you so much for your input. We will work through them thoroughly in the next weeks. Best, Werner

arfon commented 10 months ago

:wave: @wergo – how are you getting on with your edits?

wergo commented 9 months ago

We are on our way, but need some time to implement a comprehensive test framework. We are implementing this using Playwright. We are aiming to have the re-submission ready by early March.

faroit commented 9 months ago

We are on our way, but need some time to implement a comprehensive test framework. We are implementing this using Playwright. We are aiming to have the re-submission ready by early March.

@wergo great to hear about the progress. Just to clarify, you don't need to do a resubmission just updating the code or create a PR and triggering a notification here is enough for us to review the changes.

faroit commented 9 months ago

@editorialbot remind @wergo in seven days

editorialbot commented 9 months ago

Reminder set for @wergo in seven days

editorialbot commented 8 months ago

:wave: @wergo, please update us on how things are progressing here (this is an automated reminder).

wergo commented 8 months ago

We have responded to almost all issues raised by the reviewers. We are still working on a solid first set of tests to be merged. We hopefully will be able to notify you on the completed revision very soon.

faroit commented 8 months ago

@editorialbot remind @wergo in 6 days

editorialbot commented 8 months ago

Reminder set for @wergo in 6 days

editorialbot commented 8 months ago

:wave: @wergo, please update us on how things are progressing here (this is an automated reminder).

faroit commented 7 months ago

@wergo can you please update us on the status of the revisions? I see that there are lots of changes outside of the main branch, maybe you can just merge a few of them so that our reviewers can look at the changes?

wergo commented 7 months ago

@faroit Thank you for your patience!

We have now responded to all of the issues raised by the reviewers and have pushed our update code including the PlayWright test suite to our staging environment (staging branch). We plan to release the update code to production (https://mei-friend.mdw.ac.at) in the coming few days as version 1.0.12.

Regarding @rlskoeser's suggestion about a tutorial, please note that tutorial slides exist at https://tinyurl.com/mei-friend-workshop. These make extensive use of mei-friend's URL parameter remote control feature to guide users through a simple encoding session.

We would be grateful for your feedback and whether further changes are required.

All the best, @wergo and @musicog

musicog commented 7 months ago

Dear @faroit, a quick update regarding our playwright tests: they are now automatically launched by a pre-push git-hook which executes on the 'testing', 'staging', and 'main' branches (i.e., those that are served by the publicly-visible *.mei-friend.mdw.ac.at service). The hook is supplied in the repository under a new .githooks directory. The INSTALL.md file has been updated with a single command (git config) that needs to be run to enable this.

musicog commented 7 months ago

May I ask for a brief update on the review process? Is any further information or action required from our side at this stage?

stefan-balke commented 6 months ago

Hey @musicog,

thanks for the efforts. I commented on the respective issues.

Thanks for all the work and sorry for the delay, I was pretty busy in the last months, but I hope, that this review process was also a little help for you to make this tool better. It's starting to get used by the community so this is a big win!

From my side, I would be happy to proceed to publication.

musicog commented 6 months ago

@stefan-balke No need to apologise! Thank you very much (also to @rlskoeser!) for your in-depth feedback, this was very helpful to put mei-friend on a more sustainable footing. We'll continue to work further in this direction in future work.

wergo commented 6 months ago

@stefan-balke Thank you so much for your detailed feedback! This helped a lot to improve the project! All the best, Werner

faroit commented 6 months ago

@rlskoeser can you please update us on the status of your review? The authors have made recent changes, so please check back on the open issues and your checklist

rlskoeser commented 6 months ago

Thanks for pinging me, I'll try to take a look this week and see if I can sign off the remaining items on my checklist.

rlskoeser commented 6 months ago

I was able to run through the updated installation instructions and run the application, and also installed and ran the end-to-end tests. They didn't all pass for me, but I don't know if that is expected or if I'm using the correct options. The install still has a lot of manual steps that seems like that could be made easier with software package management, but I understand you made that decision deliberately so I'm ok signing off if everyone else is.

Was there an updated version of the software paper or did that not change?

musicog commented 6 months ago

@rlskoeser Thanks again for your feedback! Regarding the paper, the following updates were undertaken: https://github.com/mei-friend/mei-friend/compare/d4cb7..f6f43

In summary:

Expanded summary to more clearly identifying target audience(s) and key innovations that set the editor apart
Add info on testing environment
Updated screenshot of interface
Update visitor count
Add funding acknowledgement for Anna Plaksin's extension development
Added citation to DLfM 2023 paper ("Listen Here!")
Fixed DOI problem in another citation (DLfM 2019 "Interweaving...")
Lightly edit to retain fit to 4 pages

rlskoeser commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

rlskoeser commented 6 months ago

@musicog thanks for the summary of the revisions. The updates to the paper look great.

I'm happy to report that I've now checked off all the remaining items on my reviewer checklist.

Congratulations @wergo and @musicog on your work with mei-friend and all the success and community uptake you're seeing with it.

faroit commented 6 months ago

@musicog @wergo thanks for the update. @rlskoeser @stefan-balke thanks for the timely reviews!

We can now proceed with the post-review phase. It will take me a couple of days to re-read the paper again.

faroit commented 6 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Double check authors and affiliations (including ORCIDs)
Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
Archive the release on Zenodo/figshare/etc and post the DOI here.
Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.
Make sure that the license listed for the archive is the same as the software license.

Editor Tasks Prior to Acceptance

[x] Read the text of the paper and offer comments/corrections (as either a list or a pull request)
[x] Check that the archive title, author list, version tag, and the license are correct
[x] Set archive DOI with @editorialbot set <DOI here> as archive
[x] Set version with @editorialbot set <version here> as version
[x] Double check rendering of paper with @editorialbot generate pdf
[x] Specifically check the references with @editorialbot check references and ask author(s) to update as needed
[x] Recommend acceptance with @editorialbot recommend-accept

wergo commented 6 months ago

Dear @faroit, thanks a lot for shepherding us through this process. We will take these steps in the next days and let you know when everything is ready. Many thanks @wergo & @musicog

Dear @rlskoeser, dear @stefan-balke, thank you so much for your detailed input that helped a lot to improve this project!

faroit commented 6 months ago

@editorialbot check references

editorialbot commented 6 months ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.17613/fc1c-mx52 is OK
- 10.18061/emr.v16i1.7643 is OK
- 10.14361/9783839451458-015 is OK
- 10.1145/3543882.3543892 is OK
- 10.1145/3625135.3625144 is OK
- 10.1525/JAMS.2016.69.1.273 is OK
- 10.1145/3243907.3243911 is OK
- 10.1007/s00799-018-0262-x is OK
- 10.1145/3625135.3625149 is OK
- 10.1145/3543882.3543891 is OK
- 10.1145/3358664.3358666 is OK
- 10.1145/2872518.2890529 is OK
- 10.18716/ride.a.15.2 is OK

MISSING DOIs

- No DOI given, and none found for title: The mei-friend Web Application: Editing MEI in the...
- No DOI given, and none found for title: Linking and visualising performance data and seman...
- No DOI given, and none found for title: Verovio: A library for Engraving MEI Music Notatio...
- No DOI given, and none found for title: Digital Encoding of Music Notation with MEI
- No DOI given, and none found for title: Music Encoding Initiative (MEI) Format Family
- No DOI given, and none found for title: CodeMirror, version 5
- No DOI given, and none found for title: (Further) development of Research Tools & Data Ser...
- No DOI given, and none found for title: MEI Guidelines (4.0.1)
- No DOI given, and none found for title: MEI Guidelines (5.0.0)
- No DOI given, and none found for title: “Ain schone kunstliche Underweisung”: Modelling Ge...

INVALID DOIs

- None

openjournals / joss-reviews

[REVIEW]: mei-friend: An Interactive Web-based Editor for Digital Music Encodings #6002

Status

Reviewer instructions & questions

Checklists

Review checklist for @stefan-balke

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @rlskoeser

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

installation

community guidelines

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance