Joss-reviews: [REVIEW]: GPP, the Generic Preprocessor

Created on 28 Jun 2020 · 57Comments · Source: openjournals/joss-reviews

Submitting author: @Logological (Tristan Miller)
Repository: https://github.com/logological/gpp.git
Version: 2.27
Editor: @gkthiruvathukal
Reviewer: @Smattr, @drj11
Archive: 10.5281/zenodo.3961322

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@Smattr & @drj11, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @gkthiruvathukal know.

✨ Please try and complete your review in the next six weeks ✨

Review checklist for @Smattr

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 repository url?
[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 (@Logological) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[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?

Review checklist for @drj11

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 repository url?
[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 (@Logological) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[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?

accepted published recommend-accept review

Source

whedon

All 57 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @Smattr, @drj11 it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

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

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

whedon on 28 Jun 2020

:point_right: Check article proof :page_facing_up: :point_left:

whedon on 28 Jun 2020

Reference check summary:

OK DOIs

- 10.1007/978-3-642-37521-7_5 is OK
- 10.1109/WCRE.2001.957833 is OK
- 10.1145/363717.363746 is OK
- 10.1016/0066-4138(69)90001-9 is OK
- 10.1109/TSE.2002.1158288 is OK
- 10.1145/2211616.2211617 is OK
- 10.1093/comjnl/28.1.29 is OK
- 10.1007/978-1-4842-5149-2_10 is OK
- 10.1145/367177.367223 is OK
- 10.1145/987316.987320 is OK
- 10.1145/800197.806048 is OK
- 10.1109/IPCC.1992.673146 is OK
- 10.1093/comjnl/8.3.225 is OK
- 10.1093/comjnl/23.3.194 is OK

MISSING DOIs

- None

INVALID DOIs

- None

whedon on 28 Jun 2020

Hi @Smattr and @drj11, can you please give a brief update on progress to review this submission? Thank you!

gkthiruvathukal on 12 Jul 2020

Certainly. I made progress on a few of the initial checklist items, as indicated above. However, unfortunately I didn't have much time over the last couple of weeks to do more than that. I should have time this coming week to get deeper into the review.

Smattr on 12 Jul 2020

Thanks, @Smattr. Just was doing a gentle nudge to ensure continued progress.

gkthiruvathukal on 12 Jul 2020

👍1

A couple of questions:

GPP appears to have no test suite, which is one of the items on the review checklist. However, in my experience it is extremely challenging to define a relevant suite of test cases for a tool as generic as this. In practice, taking bug reports from your users based on the bizarre and creative things they attempt with your tool is more fruitful and representative of real world scenarios. Is this item a strict requirement? @logological, maybe you can comment on the choice to omit a test suite too?
Does Whedon check the reference syntax? The PDF output looks fine to me, but the link for that item in the checklist goes to some advice about R Markdown suggesting to me that I'm supposed to review the source rather than the PDF.

Smattr on 12 Jul 2020

👍1

@smattr: According to the review criteria, a fully automated test suite is not required. It is acceptable to provide instead "documented manual steps that can be followed to objectively check the expected functionality of the software (e.g., a sample input file to assert behaviour)". I believe that we've met this requirement by including in the distribution a rather large example input file and its output—namely, the GPP documentation itself. As we write in the README and on the GPP website,

Note that the documentation itself is generated by GPP using an input file (gpp.pp) with macros that can automatically produce HTML, LaTeX, or troff (man page) output. Studying this file should prove instructive, and manually running it through GPP can serve as a basic test of the program's functionality.

Besides this, the documentation includes a large number of example code snippets, most of which are accompanied by the expected output or a prose description thereof.

Providing an automated test suite using Automake is something that I'd eventually like to do, though this will take some effort and, given what I've written above, I didn't think it necessary to do so for the JOSS submission.

logological on 13 Jul 2020

👍1

@gkthiruvathukal to be perfectly honest it had dropped off my radar. However, I am proceeding with best speed this week.

drj11 on 13 Jul 2020

@gkthiruvathukal The checkbox for LICENSE requires specifically a LICENSE file. This repository has a COPYING file, which clearly fulfills the same purpose (it is an older, GPL-era, convention). Should I file an issue to copy the text to LICENSE? This reviewer would probably prefer that the guidelines accept either.

drj11 on 13 Jul 2020

👍1

I am giving a generous amount of wiggle room for "non-specialist audience" because I don't believe that there really are any non-specialist audiences for macro pre-processors.

drj11 on 13 Jul 2020

❤1

@drj11 Thanks for getting started! That's why I provide the gentle nudge. Regarding the LICENSE file, my interpretation is that it is required for the file to be named LICENSE. I tend to agree that there are traditions (e.g. GPL-era) but, for a journal, we want to see some degree of consistency among projects. I'm guessing a simple git mv will do the trick.

gkthiruvathukal on 13 Jul 2020

👍1

@Smattr and @logological, regarding test suites, I agree that a full test suite can be painful. However, it does seem like script that checks a number of inputs against expected outputs would be more than appropriate. Based on a read of @logological's follow up, it sounds like this is indeed the approach he has taken.

Having done a lot of work on language front ends in the past and accompanying tools (levers, parsers, ASTs), I think this approach is acceptable as long as we have a robust set of cases (inputs vs. expected outputs).

gkthiruvathukal on 13 Jul 2020

Regarding the LICENSE file, my interpretation is that it is required for the file to be named LICENSE. I tend to agree that there are traditions (e.g. GPL-era) but, for a journal, we want to see some degree of consistency among projects. I'm guessing a simple git mv will do the trick.

Yes, consistency is best. But in this case it's JOSS which is the outlier: for 30-odd years it's been a de facto standard in the C-coding free software community (and a de jure standard for GNU packages) that licensing information goes in a file called COPYING (or COPYING.LESSER for LGPL-licensed projects). The GNU Autotools, which is probably still the most common packaging system for C software on *nix, hard-code this filename convention. I can't simply rename COPYING to LICENSE without changing my build scripts; this will moreover confuse and/or irritate many users who are expecting the licence to be in the standard place, and will prevent me from submitting GPP as an official GNU package (which is something I have been mulling over for a while).

I'd ask that JOSS's rule about the licence file be interpreted liberally in this case, or else globally changed to reflect the reality that different programming languages and communities have different standards and traditions concerning the naming of the licence file.

logological on 13 Jul 2020

👍1

I agree with @logological on the COPYING/LICENSE here. While GitHub is encouraging people to add LICENSE files, we should accept long-standard community practice that share the same goals. I note that the GitHub sidebar "knows" that the project is LGPL-3.0 licensed and links to the file called COPYING. So, even though GitHub documentation tell you to create a file called LICENSE, it still accepts and knows about it if you put it in a file called COPYING.

Would a symbolic link be acceptable?

drj11 on 13 Jul 2020

@logological and @drj11, I tend to agree this request merits consideration.

@openjournals/joss-eics Please see the above regarding the LICENSE file issue. Our review guidelines don't expressly prohibit placing he license in COPYING, which is really part of the GNU/GPL tradition. What is the EICs' position on this?

gkthiruvathukal on 13 Jul 2020

It's not really JOSS that's the outlier (or at least, JOSS is not alone in this outlier), it's really "the non-GNU internet". But @logological is right about 30 years of GNU-and-others tradition.

drj11 on 13 Jul 2020

👍1

Yes, consistency is best. But in this case it's JOSS which is the outlier: for 30-odd years it's been a de facto standard in the C-coding free software community (and a de jure standard for GNU packages) that licensing information goes in a file called COPYING (or COPYING.LESSER for LGPL-licensed projects).

Oh, and just to give some quick third-party evidence of this: I surveyed the /usr/share/doc directory tree for the official packages installed on my openSUSE system; COPYING* files outnumber LICENSE* files by about two to one. Packages using COPYING are overwhelmingly C and C++ programs and libraries (but not only GNU ones), while those using LICENSE tend to be written in Ruby, Rust, Perl, etc., or else do not contain source code at all (i.e., font or dictionary packages).

logological on 13 Jul 2020

The section on licensing in the review criteria says:

Software license

There should be an OSI approved license included in the repository. Common licenses such as those listed on choosealicense.com are preferred. Note there should be an actual license file present in the repository not just a reference to the license.
Acceptable: A plain-text LICENSE file with the contents of an OSI approved license
Not acceptable: A phrase such as ‘MIT license’ in a README file

And this repository clearly meets the criteria even if it does not use a file called LICENSE. I think the checkpoint text should be changed to "Does the repository contain a plain-text license file with the contents of an OSI approved software license? Ideally in the file called LICENSE but other community-recognised names, such as COPYING, are also acceptable.".

drj11 on 13 Jul 2020

@drj11 and @logological, it looks like our discussion has already resulted in a patch! So please proceed with the rest of the review. COPYING is a valid license file now!

gkthiruvathukal on 13 Jul 2020

@gkthiruvathukal I have the same References question as @Smattr . Since ·whedon helpfully compiles the PDF, should we be looking at the references as they appear in the PDF and not the .bib file?

drj11 on 13 Jul 2020

@drj11 Yes, please check both the .bib and the PDF. If there is a problem with the entry or its rendering, it needs to be reported as an issue.

gkthiruvathukal on 13 Jul 2020

I'm done with my review. It was a pleasure to read this submission -- very informative and also high quality. I wish I'd known more about GPP previously, and it's definitely something I'll consider using in future.

Smattr on 19 Jul 2020

I have checked paper.bib using https://biblatex-linter.herokuapp.com/validate which notes that 4 entries are missing an institution. In my opinion this is of no consequence.

drj11 on 21 Jul 2020

@gkthiruvathukal i have completed my review.

drj11 on 21 Jul 2020

I have checked paper.bib using https://biblatex-linter.herokuapp.com/validate which notes that 4 entries are missing an institution. In my opinion this is of no consequence.

This is because that validator expects its input to use the biblatex dialect, whereas my bibliography entries are written in the older BibTeX dialect. Whatever program JOSS is using to compile the paper is correctly recognizing the entries as BibTeX and is properly including the institutional information (which is in the school field).

logological on 21 Jul 2020

@logological ah, that is reassuring. I was wondering how I, having last touched a bitex file in 1992, was expected to know the difference between the two. :)

drj11 on 21 Jul 2020

I was wondering myself which dialect to use for the bibliography, since I don't think this is covered in the submission instructions. I decided it was safer to go with BibTeX since the modern biblatex bibliographic processor can handle both biblatex and BibTeX dialects, whereas the older BibTeX processor doesn't understand biblatex. Perhaps this could be clarified in the JOSS style guide so that authors don't have to guess like I did.

logological on 21 Jul 2020

👍1

@gkthiruvathukal Should I make a new tagged release of GPP incorporating the documentation changes resulting from the review? Or are there any further changes you think are necessary before you think this submission can be accepted for publication?

logological on 25 Jul 2020

@logological Yes, it looks like we are ready for the next steps. Please do the following:

[x] Make a tagged release of your software, and list the version tag of the archived version here. [That answers your question above!]
[x] Archive the reviewed software in Zenodo
[x] Check the Zenodo deposit has the correct metadata, this includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it); you may also add the authors' ORCID.
[x] List the Zenodo DOI of the archived version here.

gkthiruvathukal on 26 Jul 2020

@gkthiruvathukal:
OK, the new release tag in GitHub is as follows: 2.27
The Zenodo DOI is as follows: