feat: Add function to check whether all versions are git-tagged#404
feat: Add function to check whether all versions are git-tagged#404
Conversation
krlmlr
left a comment
krlmlr
left a comment
There was a problem hiding this comment.
Thanks! Fine with adding code and keeping it private for now.
https://github.com/cynkra/fledge/actions/runs/3821539430/jobs/6500786019#step:12:178 |
There was a problem hiding this comment.
Pull request overview
Adds a new synchronization check to ensure every version listed in NEWS.md has a corresponding Git tag, addressing #403 (“NEWS -> tags (warn if tags are missing)”).
Changes:
- Introduces
check_tags()to compareNEWS.mdversions against Git tags and warn on missing tags. - Adds snapshot-based test coverage for the new behavior.
- Adds generated documentation and a runnable example for the new function.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
R/synchronization.R |
Implements check_tags() that warns when NEWS versions lack tags. |
tests/testthat/test-synchronization.R |
Adds snapshot + silent-path tests for check_tags(). |
tests/testthat/_snaps/synchronization.md |
Stores expected warning/output snapshot for the missing-tag case. |
man/examples/check-tags.R |
Adds an example workflow demonstrating check_tags(). |
man/check_tags.Rd |
Adds generated Rd documentation for check_tags(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #' Check whether all versions NEWS.md have been git-tagged | ||
| #' | ||
| #' @return `NULL` | ||
| #' |
There was a problem hiding this comment.
check_tags() is documented, tested, and used in examples via fledge::check_tags(), but it is not exported (no @export tag). As-is, fledge::check_tags() will fail and check_tags() won’t be found when running tests against an installed package. Add @export in the roxygen block (and regenerate NAMESPACE via roxygen2).
| #' | |
| #' | |
| #' @export |
| @@ -0,0 +1,23 @@ | |||
| #' Check whether all versions NEWS.md have been git-tagged | |||
There was a problem hiding this comment.
The title/description text is grammatically off: “all versions NEWS.md” is missing “in” (i.e., “all versions in NEWS.md”). Since man/check_tags.Rd is generated from this roxygen block, fix the wording here and re-run roxygen so the Rd stays in sync.
| #' Check whether all versions NEWS.md have been git-tagged | |
| #' Check whether all versions in NEWS.md have been git-tagged |
| # Bump version with fledge. | ||
| fledge::bump_version() | ||
| fledge::update_news(c("- something I forgot", "- blabla")) | ||
| fledge::update_version() |
There was a problem hiding this comment.
This example calls fledge::update_version(), which is marked deprecated in the package docs. Examples are executed during R CMD check, so it’s better to avoid deprecated APIs here (use fledge::update_news() instead).
| fledge::update_version() |
| versions <- get_news_headers()[["version"]] | ||
| tags <- gsub("^v", "", gert::git_tag_list()[["name"]]) | ||
|
|
||
| untagged_versions <- versions[!(versions %in% tags)] |
There was a problem hiding this comment.
| untagged_versions <- versions[!(versions %in% tags)] | |
| untagged_versions <- setdiff(versions, tags) |
| } | ||
|
|
||
| rlang::warn( | ||
| sprintf( |
There was a problem hiding this comment.
the package imports cli so this should use cli::cli_warn() and cli pluralization. https://cli.r-lib.org/reference/pluralization.html
Fix #403