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

More upkeep polishing #1909

Merged
merged 5 commits into from
Nov 13, 2023
Merged

More upkeep polishing #1909

merged 5 commits into from
Nov 13, 2023

Conversation

hadley
Copy link
Member

@hadley hadley commented Oct 31, 2023

No description provided.

@hadley hadley changed the title Also update favicons More upkeep polishing Oct 31, 2023
@olivroy
Copy link
Contributor

olivroy commented Oct 31, 2023

I don't know if it is related, but maybe removing unnecessary suggests dependencies such as crayon, covr, ellipsis, roxygen2, devtools etc. would be good practice since github actions handles test coverage, and covr is installed via gh action specifically.

@hadley
Copy link
Member Author

hadley commented Oct 31, 2023

@olivroy this is specifically for our internal upkeep process, where I don't think unneeded dependencies tend to be much of a problem.

@hadley hadley requested a review from jennybc November 6, 2023 14:58
@@ -112,8 +112,6 @@ release_checklist <- function(version, on_cran) {
Check if any deprecation processes should be advanced, as described in \\
[Gradual deprecation](https://lifecycle.r-lib.org/articles/communicate.html#gradual-deprecation)",
type != "patch" && has_lifecycle),
todo("Bump required R version in DESCRIPTION to {tidy_min_r_version}",
Copy link
Member Author

Choose a reason for hiding this comment

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

Now in upkeep, rather than release.

Copy link
Member

Choose a reason for hiding this comment

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

I kinda like it as part of the release checklist, but not a hill I will die on.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have strong feelings either except that it should be in one or the other.

expect_snapshot(
writeLines(tidy_upkeep_checklist(posit_pkg = TRUE, posit_person_ok = FALSE)),
transform = scrub_checklist_footer
local_mocked_bindings(
Copy link
Member Author

Choose a reason for hiding this comment

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

I probably should've done this in a separate PR, but using mocking for everything makes the tests quite a bit simpler.

@@ -112,8 +112,6 @@ release_checklist <- function(version, on_cran) {
Check if any deprecation processes should be advanced, as described in \\
[Gradual deprecation](https://lifecycle.r-lib.org/articles/communicate.html#gradual-deprecation)",
type != "patch" && has_lifecycle),
todo("Bump required R version in DESCRIPTION to {tidy_min_r_version}",
Copy link
Member

Choose a reason for hiding this comment

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

I kinda like it as part of the release checklist, but not a hill I will die on.

@hadley hadley merged commit 2fcc92c into main Nov 13, 2023
13 checks passed
@hadley hadley deleted the more-release-polishing branch November 13, 2023 23:01
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.

3 participants