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

feat: Add a field to DESCRIPTION to notify which version of rextendr the package was created with #225

Merged
merged 12 commits into from
Jan 6, 2023

Conversation

eitsupi
Copy link
Contributor

@eitsupi eitsupi commented Jan 4, 2023

Close #222

Like roxygen2 (r-lib/roxygen2#338), record the version of the rextendr used to create the package in DESCRIPTION to prevent the use of older versions.

It seems that the field name for this needs to start with Config/ ?
https://github.com/wch/r-source/blob/d1618921e670f1607da77ba850949f8312abdff6/src/library/tools/R/QC.R#L7204-L7213

@eitsupi eitsupi force-pushed the add-note-to-description branch from 1d03e47 to 29b78cd Compare January 5, 2023 04:07
@eitsupi eitsupi marked this pull request as ready for review January 5, 2023 04:07
@eitsupi

This comment was marked as resolved.

@Ilia-Kosenkov
Copy link
Member

Ok, I tested it locally, seems to work fine. However, I am not entirely sure about the path in the config name. Is this because we want to avoid R CMD check checking this field? Why Roxygen can have their own name while we cannot?

@yutannihilation
Copy link
Contributor

Is this because we want to avoid R CMD check checking this field?

In my understanding, we don't really need this, but it's a common practice among tidyverse packages to use Config/ nowadays. In theory, we can put any field as the WRE says:

There is no restriction on the use of other fields not mentioned here (but using other capitalizations of these field names would cause confusion). Fields Note, Contact (for contacting the authors/developers9) and MailingList are in common use. Some repositories (including CRAN and R-forge) add their own fields.

It seems common field names like Roxygen are manually excluded: https://github.com/wch/r-source/blob/65710ee970c905903c497e806194b41b04385562/src/library/tools/R/utils.R#L1364-L1375. I guess the reason why roxygen2 has its own field is that it was born long before Config/ is popular.

@Ilia-Kosenkov
Copy link
Member

Ok, sounds fair. @eitsupi, let me know if you are finished with this PR -- I need to force-merge it to bypass 32-bit failing CI.

@eitsupi
Copy link
Contributor Author

eitsupi commented Jan 6, 2023

Thank you all for your help.

@eitsupi, let me know if you are finished with this PR -- I need to force-merge it to bypass 32-bit failing CI.

The work is done and I would appreciate it if you could merge it if the field name is ok.
(The field name is obviously difficult to change later once release!)

@Ilia-Kosenkov Ilia-Kosenkov merged commit 870a03a into extendr:main Jan 6, 2023
@eitsupi eitsupi deleted the add-note-to-description branch January 6, 2023 22:36
@Ilia-Kosenkov
Copy link
Member

Thank you for contributing @eitsupi !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notify which version of rextendr the package was created with
4 participants