Skip to content
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]: mei-friend: An Interactive Web-based Editor for Digital Music Encodings #6002

Open
editorialbot opened this issue Nov 1, 2023 · 40 comments
Assignees
Labels
CSS JavaScript Python review Track: 4 (SBCS) Social, Behavioral, and Cognitive Sciences

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Nov 1, 2023

Submitting author: @wergo (Werner Goebl)
Repository: https://github.com/mei-friend/mei-friend
Branch with paper.md (empty if default branch): joss2023
Version: v1.0.0
Editor: @faroit
Reviewers: @stefan-balke, @rlskoeser
Archive: Pending

Status

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Wordcount for paper.md is 1276

@editorialbot
Copy link
Collaborator Author

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

faroit commented Nov 1, 2023

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@faroit
Copy link

faroit commented Nov 1, 2023

@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
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@stefan-balke
Copy link

stefan-balke commented Nov 1, 2023

Review checklist for @stefan-balke

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the https://github.com/mei-friend/mei-friend?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • 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?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • 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.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • 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

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

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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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
Copy link

rlskoeser commented Nov 1, 2023

Review checklist for @rlskoeser

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the https://github.com/mei-friend/mei-friend?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • 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?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • 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.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • 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

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

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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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
Copy link

wergo commented Nov 1, 2023

@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
Copy link

stefan-balke commented Nov 2, 2023

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

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

@wergo
Copy link

wergo commented Nov 10, 2023

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

@faroit
Copy link

faroit commented Nov 10, 2023

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

@rlskoeser
Copy link

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

rlskoeser commented Nov 17, 2023

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

faroit commented Nov 18, 2023

@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
Copy link

@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
Copy link

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

@wergo
Copy link

wergo commented Dec 4, 2023

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

@arfon
Copy link
Member

arfon commented Feb 4, 2024

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

@wergo
Copy link

wergo commented Feb 14, 2024

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

faroit commented Feb 19, 2024

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

faroit commented Mar 3, 2024

@editorialbot remind @wergo in seven days

@editorialbot
Copy link
Collaborator Author

Reminder set for @wergo in seven days

@editorialbot
Copy link
Collaborator Author

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

@wergo
Copy link

wergo commented Mar 12, 2024

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

faroit commented Mar 21, 2024

@editorialbot remind @wergo in 6 days

@editorialbot
Copy link
Collaborator Author

Reminder set for @wergo in 6 days

@editorialbot
Copy link
Collaborator Author

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

@faroit
Copy link

faroit commented Apr 15, 2024

@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
Copy link

wergo commented Apr 16, 2024

@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
Copy link

musicog commented May 8, 2024

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

musicog commented May 8, 2024

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

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

musicog commented May 11, 2024

@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
Copy link

wergo commented May 12, 2024

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

@faroit
Copy link

faroit commented May 13, 2024

@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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS JavaScript Python review Track: 4 (SBCS) Social, Behavioral, and Cognitive Sciences
Projects
None yet
Development

No branches or pull requests

7 participants