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

Public upkeep checklist #1825

Merged
merged 45 commits into from
Apr 26, 2023
Merged

Public upkeep checklist #1825

merged 45 commits into from
Apr 26, 2023

Conversation

ateucher
Copy link
Collaborator

@ateucher ateucher commented Apr 13, 2023

This adds a public function use_upkeep_issue() similar to use_tidy_upkeep_issue() for general (i.e. non-tidyverse) use.

I factored out the internals of use_tidy_upkeep_issue() into make_upkeep_issue() which is called by both exported functions.

I created a default checklist for use_upkeep_issue() with some good practices... I would love some feedback on what should be included there. Many of them are one-time only fixes, but most are conditionally populated so should only show up when needed.

I did also add the ability to add custom upkeep tasks similar to use_release_checklist().

Closes #1794.

@ateucher ateucher requested a review from jennybc April 13, 2023 17:05
Copy link
Member

@jennybc jennybc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some comments to move things along and start some discussion.

I'm not sure if tidy-upkeep.R is the right file for this anymore, although I'm not sure it's not. But worth thinking about.

R/tidy-upkeep.R Outdated Show resolved Hide resolved
R/tidy-upkeep.R Outdated Show resolved Hide resolved
R/tidy-upkeep.R Outdated Show resolved Hide resolved
R/tidy-upkeep.R Outdated Show resolved Hide resolved
R/tidy-upkeep.R Outdated Show resolved Hide resolved
R/tidy-upkeep.R Outdated Show resolved Hide resolved
tests/testthat/_snaps/tidy-upkeep.md Outdated Show resolved Hide resolved
tests/testthat/test-tidy-upkeep.R Outdated Show resolved Hide resolved
tests/testthat/test-tidy-upkeep.R Outdated Show resolved Hide resolved
tests/testthat/test-tidy-upkeep.R Outdated Show resolved Hide resolved
@ateucher
Copy link
Collaborator Author

ateucher commented Apr 17, 2023

I made some comments to move things along and start some discussion.

I'm not sure if tidy-upkeep.R is the right file for this anymore, although I'm not sure it's not. But worth thinking about.

I was thinking of just doing rename_files("tidy-upkeep", "upkeep"). Or do you want to separate the tidy checklist from the public checklist? It looks like currently there are some (most?) use_tidy_*() functions in tidyverse.R, and some with the parent functions (e.g., use_tidy_github_labels().

@jennybc
Copy link
Member

jennybc commented Apr 17, 2023

As long as you give the file(name) issue some thought, I'm OK with what you choose. Let's just make sure we aren't introducing some new convention. Do what seems most consistent with what we've done for other functions with generic and tidy versions. (This could happen for use_release_issue() at some point as well.)

Copy link
Member

@jennybc jennybc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not quite done, but I think we're in agreement on what "done" means.

This should also get a NEWS bullet.

R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
ateucher and others added 6 commits April 18, 2023 09:10
Co-authored-by: Jennifer (Jenny) Bryan <[email protected]>
* for `use_tidy_upkeep_issue()`, if `year` is specified it will be used in the title and used to select the subset of checklist items included (as before). If `year` is `NULL`, the current year will be in the issue title, but all current and previous checklists will be included.
* for `use_upkeep_issue()` `year` is only used in the title, as there are not multiple year-based checklists.
@ateucher
Copy link
Collaborator Author

I think this is almost done now @jennybc.

An example of a non-tidy issue: https://github.com/ateucher/rmapshaper/issues/149
And a tidy issue:#1827

R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Show resolved Hide resolved
Co-authored-by: Jennifer (Jenny) Bryan <[email protected]>
@ateucher ateucher requested a review from hadley April 25, 2023 16:19
tests/testthat/_snaps/upkeep.md Show resolved Hide resolved
tests/testthat/_snaps/upkeep.md Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
R/upkeep.R Outdated Show resolved Hide resolved
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.

Create use_upkeep_issue() to facilitate Spring Cleaning by package developers
3 participants