inbo / checklist

An R package for checking R packages and R code
https://inbo.github.io/checklist
GNU General Public License v3.0
19 stars 2 forks source link

DESCRIPTION #7

Closed ThierryO closed 1 year ago

ThierryO commented 4 years ago

Items marked with * can be ignored when motivated.

ElsLommelen commented 4 years ago

Maybe 'Add ORCID for INBO authors and contributors'?

ThierryO commented 4 years ago

Creating an ORCID is free and open for everyone. So we can require it for all authors and contributors (= people who write part of the code). Or is there a scenario that I'm missing?

I'll update the item about the ORCID

ElsLommelen commented 4 years ago

Yes, everyone indeed can, but what is the advantage of forcing people to make an ORCID for contributing to one package if they have no intention to add information to it or use it for other purposes? If they would like to use it in some years, they have probably forgotten they once made one, so they make a new one (if possible), at least that's what people do on social media when they buy a new computer and don't remember their password... Also, not everyone is eager to have his/her information on the internet.

Anyway, I see no advantage of requiring a ORCID for external contributors. As a package responsible I would certainly ask them to add it and explain what it is if they don't know, but I don't feel like forbidding them to contribute only because they are not interested in making an ORCID.

So I think you should either restrict it to INBO authors and contributors, either again add the option to ignore when motivated. Or you could do both: require for INBO (without option to ignore without motivation) and allow a motivated exception for external contributors.

ThierryO commented 4 years ago

Since these are our repositories, we can set the rules. And we put them in a CONTRIBUTING.md for everyone to read.

Here is why ROpenSci strongly recommend to use ORCID.

peterdesmet commented 1 year ago

I'm currently getting an error for a package where language is not defined (not sure that is because it assumes it is a project?):

> checklist::check_description()
No `checklist.yml` found. Assuming this is a project.
Error: Could not find DESCRIPTION field: ‘Language’.

Many packages won't have this field, it's not required if documentation is in English:

A ‘Language’ field can be used to indicate if the package documentation is not in English: this should be a comma-separated list of standard (not private use or grandfathered) IETF language tags as currently defined by RFC 5646 (https://tools.ietf.org/html/rfc5646, see also https://en.wikipedia.org/wiki/IETF_language_tag), i.e., use language subtags which in essence are 2-letter ISO 639-1 (https://en.wikipedia.org/wiki/ISO_639-1) or 3-letter ISO 639-3 (https://en.wikipedia.org/wiki/ISO_639-3) language codes.

So I would assume en-GB when undefined.

ThierryO commented 1 year ago

checklist is an opiniated set of rules and more strict than the rules which CRAN enforces. We prefer to make things explicit. Not enforcing this would allow people to write their package documentation in a different language without specifying the language. E.g. Dutch package documentation without the Language: nl-BE tag. The current settings forces the author to specify the language. They still could do silly things like setting Language: en-GB on a package with Dutch documentation. However that would results in errors when running check_spelling().

peterdesmet commented 1 year ago

I understand. I also think putting more strict requirements affects how many people will use the package. E.g. if I understand correctly, one can now no longer use write_zenodo_json() because DESCRIPTION does not have a language attribute? Or because the package does not have a checklist.yml (see #96)?

Reducing that scope of users is a valid decision, I just want to make sure it's an intended one. 😄

ThierryO commented 1 year ago

The .zenodo.json gets its language information for the Language tag in DESCRIPTION. In the new version (#94) you only need update_citation() to update all citation metadata file (.zenodo.json, CITATION.cff and in case of a package inst/CITATION). Checklist assumes to work on INBO packages. E.g. not mentioning INBO as copyright holder will yield a warning. Which you can allow by writing the checklist.yml. We strongly recommend to run checklist::setup_package() or checklist::setup_project() on existing code. And use checklist::write_checklist() to store the allowed warnings and notes.

Running any checklist::check_* functions will use "empty" checklist settings with a message.

Is checklist usable outside of an INBO context? Yes. Provided you are willing to abide the same set of rules and willing to allow some of the specific INBO related warnings and rules. E.g. you don't have to set INBO as copyright holder, but should allow that warning (and motivate why). Have a look at thierryo/qrcode