Beuth-Erdelt/DBMS-Benchmarker

JOSS Review - Documentation

erik-whiting opened this issue · 3 comments

Hi, I'm Erik Whiting, I'm one of the reviewers for your submission to the Journal of Open Source Software (JOSS). The review is being coordinated here: openjournals/joss-reviews#4628

This issue is to note my review of the documentation portion of your submission. For your convenience, I have copy and pasted the criteria set forth by the JOSS editors and my notes on why I think your paper does or does not meet the requirements. Please note, this is for the documentation review portion only.

The quoted parts are the criteria set forth by JOSS and the text under is why I think your paper does or does not meet the criteria at this time.

A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

While this isn't as explicitly stated in the documentation as it is in the paper, I think the README is sufficient enough for this bullet point. The software paper could have a variety of readers, but someone who landed on this repo probably already had an idea of what they were looking for.

Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

Yes! Installation is easy and instructions are listed in the README

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

Yes, there are lots of good examples.

Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?

Yes, the documentation is superb. Nice work!

Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?

I don't see any automated tests or manual test steps. I could be missing them, but there does not seem to be any, nor does there seem to be instructions for contributors on how run automated tests or steps for performing manual tests.

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

  1. I cannot find any guidance for potential contributors. If users of this system find bugs or want to add features, they may come to your repository to figure out what steps they need to take to chat with you about proposed changes and your general pull-request policy. Typically, this information will be contained in a CONTRIBUTING.md markdown file that contains information about how you want potential contributors to branch, commit, test, and generally participate in the development of the system. Take a look at biopython's contributing guide for a simple but thorough example.

  2. There doesn't seem to be any guidance to users who want to open issues or report problems. Although this is GitHub and there is a well-documented issue tracking system, it's possible people who don't know how to use GitHub will wind up on this page. Leaving some information about how to report bugs will be helpful to them. It doesn't need to be an exhaustive guide, just enough to point people in the right direction.

  3. There doesn't seem to be any guidance or instructions for people looking to get help using the system. The documentation is great, but people may have questions or issues not covered by the documentation. It would be helpful to them to have instructions about how they can get a hold of someone who can help them.

Hi @erik-whiting, thank you for your review!

I fixed the following: link to bug tracker and contribution guidelines (good idea btw!)

I'm unsure how to provide automated tests (there is currently no hint in the docs). Without a DBMS, the tool makes no sense. Do you have any idea? What would be appropriate? A check that certain results would be expected if installed correctly? Something like exit code = 0?

A check that certain results would be expected if installed correctly? Something like exit code = 0?

@perdelt I think this would be perfect for users. Regarding contributors:

Imagine I opened a pull-request against DBMS-Benchmarker, what steps would you go through to make sure my PR didn't break the whole system? When I learned this term, it was called a "smoke test", I don't know if that's still the common terminology. But basically, is there a series of steps you go through to make sure the software hasn't broken since the last time you updated it? If so, document that (maybe "a check that cerrtain results would be expected if installed correctly" is good enough).

Does that help?

Hi @erik-whiting

Yes, thanks, that helps a lot.

I was very focused on how to automate testing, which is very difficult.
(This would require an automated installation of a reproducible testbed, something my other package that is mentioned in the README is about).
At least when evaluating a given result, automated tests can be sensible. I will think about how to include this in future. Currently I just follow your hint.
Listing the ingredients that facilitate testing (and what I would use) is a good idea. I've added the following to the CONTRIBUTIONS page:

https://github.com/Beuth-Erdelt/DBMS-Benchmarker/blob/v0.12.5/docs/CONTRIBUTING.md

Testing

New features or functions will not be accepted without testing.
Likewise for any enhancement or bug fix, we require including an additional test.

If the feature or functionality concerns benchmarking

We expect it to be testable with the TPC-H workload.
Otherwise please add references to data and queries you used to test the functionality.
They must be publicly accessible.

If the feature or functionality concerns evaluation

Please include a zipped result folder, so we can trace the enhancement of evaluations based on the same results you have used.
(And please be sure to not include secret passwords in the connection information).