diff --git a/.github/workflows/auto-changelog.yml b/.github/workflows/auto-changelog.yml new file mode 100644 index 0000000..78cb08a --- /dev/null +++ b/.github/workflows/auto-changelog.yml @@ -0,0 +1,154 @@ +name: auto-changelog + +on: + workflow_call: + secrets: + ANTHROPIC_API_KEY: + required: true + PRIVATE_ACCESS_TOKEN: + required: true + +jobs: + auto-changelog: + runs-on: ubuntu-24.04 + permissions: + contents: write + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + token: ${{ secrets.PRIVATE_ACCESS_TOKEN }} + ref: ${{ github.head_ref }} + + - name: Check if version bump and NEWS.md entry are present + id: check + run: | + PKG_NAME=$(grep "^Package:" DESCRIPTION | sed 's/Package: //') + CURRENT_VERSION=$(grep "^Version:" DESCRIPTION | sed 's/Version: //') + MAIN_VERSION=$(git show origin/main:DESCRIPTION | grep "^Version:" | sed 's/Version: //') + + echo "pkg_name=$PKG_NAME" >> "$GITHUB_OUTPUT" + echo "current_version=$CURRENT_VERSION" >> "$GITHUB_OUTPUT" + echo "main_version=$MAIN_VERSION" >> "$GITHUB_OUTPUT" + + VERSION_BUMPED=false + NEWS_OK=false + + if [ "$CURRENT_VERSION" != "$MAIN_VERSION" ]; then + VERSION_BUMPED=true + fi + + if grep -q "^## $PKG_NAME $CURRENT_VERSION" NEWS.md; then + NEWS_OK=true + fi + + if [ "$VERSION_BUMPED" = "true" ] && [ "$NEWS_OK" = "true" ]; then + echo "skip=true" >> "$GITHUB_OUTPUT" + else + echo "skip=false" >> "$GITHUB_OUTPUT" + fi + + - name: Generate version bump and NEWS entry via Claude + if: steps.check.outputs.skip == 'false' + id: claude + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + PKG_NAME="${{ steps.check.outputs.pkg_name }}" + MAIN_VERSION="${{ steps.check.outputs.main_version }}" + + # Limit diff to 6000 chars to stay within token limits; write to file to avoid shell injection + git diff origin/main...HEAD -- . \ + ':(exclude)NEWS.md' \ + ':(exclude)DESCRIPTION' \ + ':(exclude)*.lock' \ + ':(exclude)*.rdb' \ + | head -c 6000 > /tmp/pr_diff.txt + + PKG_NAME_ENV="$PKG_NAME" MAIN_VERSION_ENV="$MAIN_VERSION" python3 - < NEWS.md.tmp && mv NEWS.md.tmp NEWS.md + + - name: Commit and push + if: steps.check.outputs.skip == 'false' + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + git add DESCRIPTION NEWS.md + git commit -m "chore: bump version and update NEWS.md" + git push diff --git a/.github/workflows/conventional-commit-check.yml b/.github/workflows/conventional-commit-check.yml new file mode 100644 index 0000000..01fba2f --- /dev/null +++ b/.github/workflows/conventional-commit-check.yml @@ -0,0 +1,23 @@ +name: conventional-commit-check + +on: + workflow_call: + +jobs: + check-pr-title: + runs-on: ubuntu-24.04 + steps: + - name: Check PR title format + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: | + PATTERN='^(feat|fix|refactor|docs|test|chore|ci|perf|build|revert)(\([^)]+\))?: .+' + if echo "$PR_TITLE" | grep -qE "$PATTERN"; then + echo "PR title follows Conventional Commits: '$PR_TITLE'" + else + echo "::warning::PR title '$PR_TITLE' does not follow Conventional Commits format." + echo "::warning::Expected: : (e.g. 'feat: add new linter')" + echo "::warning::Valid types: feat, fix, refactor, docs, test, chore, ci, perf, build, revert" + echo "::warning::See the style guide vignette for details. This check is informational only." + fi + exit 0 diff --git a/DESCRIPTION b/DESCRIPTION index 60d27aa..ba2f939 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,8 +1,8 @@ Package: gDRstyle Type: Package Title: A package with style requirements for the gDR suite -Version: 1.11.2 -Date: 2026-05-05 +Version: 1.11.4 +Date: 2026-05-08 Authors@R: c( person("Allison", "Vuong", role=c("aut")), person("Dariusz", "Scigocki", role=c("aut")), @@ -27,7 +27,8 @@ Imports: checkmate, desc, git2r, - lintr (>= 3.0.0), + cyclocomp, + lintr (>= 3.2.0), rcmdcheck, remotes, yaml, diff --git a/NAMESPACE b/NAMESPACE index 9d5db71..823502e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -5,6 +5,7 @@ export(checkDependencies) export(checkPackage) export(installAllDeps) export(installLocalPackage) +export(lintNewsEntries) export(lintPkgDirs) export(roxygen_tag_linter) importFrom(BiocStyle,html_document) diff --git a/NEWS.md b/NEWS.md index a9f8a6f..52142aa 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,11 @@ +## gDRstyle 1.11.4 - 2026-05-18 +* apply updated gDRstyle rules + +## gDRstyle 1.11.3 - 2026-05-08 +* extend linter config with additional rules and update style guide accordingly +* add auto-changelog GitHub Actions reusable workflow using Claude API +* add lintNewsEntries to enforce brevity and style in NEWS.md entries + ## gDRstyle 1.11.2 - 2026-05-05 * support vector notation for `length` field in `note.json` @@ -176,4 +184,4 @@ * fix WARNINGS and NOTES in check- ## gDRstyle 0.1.3.9 - 2023-01-05 -* the note test is stricter +* the note test is stricter \ No newline at end of file diff --git a/R/build_tools.R b/R/build_tools.R index b5391fc..c2ef972 100644 --- a/R/build_tools.R +++ b/R/build_tools.R @@ -18,7 +18,7 @@ setReposOpt <- function(additionalRepos = NULL) { #' @return \code{NULL} or info about error #' @keywords internal #' @noRd -setTokenVar <- function(base_dir, +setTokenVar <- function(base_dir, filename = ".github_access_token.txt") { # Use GitHub access_token if available gh_access_token_file <- file.path(base_dir, filename) @@ -51,7 +51,7 @@ setTokenVar <- function(base_dir, #' @return \code{NULL} or info about error #' @keywords internal #' @noRd -verify_version <- function(name, +verify_version <- function(name, required_version) { pkg_version <- utils::packageVersion(name) @@ -67,24 +67,24 @@ verify_version <- function(name, is_version_ok <- function(pkg_ver, req) { valid_ver_regex <- .standard_regexps()$valid_numeric_version - + req <- if (grepl(paste0("^", valid_ver_regex, "$"), req)) { paste0("==", req) } else { req } - + fun <- trimws(sub("\\d.*", "", req)) req_ver <- trimws(sub(fun, "", req)) - + if (!grepl("==|<|>|<=|>=", fun)) stop("Invalid comparison operator") get(fun, mode = "function")(pkg_ver, req_ver) } #' Create a new ssh key credential object -#' -#' @param use_ssh logical, if use ssh keys +#' +#' @param use_ssh logical, if use ssh keys #' #' @return A list of class cred_ssh_key #' @keywords internal @@ -108,7 +108,7 @@ getSshKeys <- function(use_ssh) { #' installLocalPackage(system.file( #' package = "gDRstyle", "tst_pkgs", "dummy_pkg" #' )) -#' +#' #' @return \code{NULL} #' @keywords install #' @export @@ -189,7 +189,7 @@ installAllDeps <- function(additionalRepos = NULL, #' @return \code{NULL} #' @keywords internal #' @noRd -install_cran <- function(name, +install_cran <- function(name, pkg) { if (is.null(pkg$repos)) { pkg$repos <- getOption("repos") @@ -211,7 +211,7 @@ install_cran <- function(name, #' @return \code{NULL} #' @keywords internal #' @noRd -install_bioc <- function(name, +install_bioc <- function(name, pkg) { if (is.null(pkg$ver)) { pkg$ver <- BiocManager::version() @@ -232,18 +232,18 @@ install_bioc <- function(name, #' @return \code{NULL} #' @keywords internal #' @noRd -install_github <- function(name, +install_github <- function(name, pkg) { if (is.null(pkg$ref)) { pkg$ref <- "HEAD" } - + host_url <- if (!is.null(pkg$host)) { pkg$host } else { "api.github.com" } - + remotes::install_github( repo = pkg$url, ref = pkg$ref, @@ -264,8 +264,8 @@ install_github <- function(name, #' @return \code{NULL} #' @keywords internal #' @noRd -install_git <- function(name, - pkg, +install_git <- function(name, + pkg, keys) { remotes::install_git( url = pkg$url, @@ -286,7 +286,7 @@ install_git <- function(name, #' @return \code{NULL} #' @keywords internal #' @noRd -install_gitlab <- function(name, +install_gitlab <- function(name, pkg) { repo <- paste(tempdir(), "install_pkg_git", name, sep = .Platform$file.sep) url <- if (!is.null(pkg$url) && grepl("code.roche.com", pkg$url)) { diff --git a/R/check.R b/R/check.R index ec2e432..a65a7a8 100644 --- a/R/check.R +++ b/R/check.R @@ -55,7 +55,7 @@ test_notes_check <- function(check_results, if (!all(is_note_valid)) { stop("Check found unexpected NOTEs: \n", - paste0(check_results$notes[!is_note_valid], collapse = " ")) + paste(check_results$notes[!is_note_valid], collapse = " ")) } } } @@ -233,6 +233,7 @@ checkPackage <- function(pkgName, utils::timestamp() with_shiny <- file.exists(file.path(pkgDir, "inst", "shiny")) gDRstyle::lintPkgDirs(pkgDir, shiny = with_shiny) + gDRstyle::lintNewsEntries(pkgDir) } else { message("Lint skipped") } diff --git a/R/dependencies.R b/R/dependencies.R index b555b6f..92a9519 100644 --- a/R/dependencies.R +++ b/R/dependencies.R @@ -14,7 +14,7 @@ #' #' @examples #' checkDependencies( -#' dep_path = +#' dep_path = #' system.file(package = "gDRstyle", "testdata", "dependencies.yaml"), #' desc_path = system.file(package = "gDRstyle", "DESCRIPTION"), #' skip_pkgs = c("testthat", "lintr") @@ -71,7 +71,7 @@ checkDependencies <- function(dep_path, sprintf(avoid_new_lines( "misaligned package versions between 'rplatform/dependencies.yaml' and package 'DESCRIPTION' file: %s"), - paste0(bad_pkgs, collapse = ", ") + paste(bad_pkgs, collapse = ", ") ) ) } @@ -92,7 +92,7 @@ checkDependencies <- function(dep_path, #' @return Character vector of any misaligned package versions between #' rplatform \code{dependencies.yaml} and package \code{DESCRIPTION}. #' @keywords internal -compare_versions <- function(rp, +compare_versions <- function(rp, desc) { stopifnot(all(names(rp) == names(desc))) misaligned_ver_pkgs <- NULL @@ -120,7 +120,7 @@ compare_versions <- function(rp, #' @keywords internal #' @noRd -get_all_pkgs <- function(combo_path, +get_all_pkgs <- function(combo_path, rp_pkgs) { if (file.exists(combo_path)) { combo_deps <- yaml::read_yaml(combo_path) @@ -138,14 +138,14 @@ get_all_pkgs <- function(combo_path, #' @keywords internal #' @noRd -pkgs_search <- function(rp_ver, +pkgs_search <- function(rp_ver, desc_deps) { idx <- match(names(rp_ver), desc_deps$package) if (any(na_idx <- is.na(idx))) { stop(sprintf(avoid_new_lines( "packages specified in 'dependencies.yaml' not present in 'DESCRIPTION': %s"), - paste0(names(rp_ver)[na_idx], collapse = ", ") + paste(names(rp_ver)[na_idx], collapse = ", ") )) } xrp_ver <- desc_deps[idx, "version"] @@ -156,8 +156,8 @@ pkgs_search <- function(rp_ver, #' @keywords internal #' @noRd -pkgs_reverse_search <- function(desc, - skip, +pkgs_reverse_search <- function(desc, + skip, all) { cond <- desc[["version"]] != "*" & !desc[["package"]] %in% skip pkgs <- desc[cond, c("package")] diff --git a/R/gDRstyle-package.R b/R/gDRstyle-package.R index e484dcf..b37ca66 100644 --- a/R/gDRstyle-package.R +++ b/R/gDRstyle-package.R @@ -1,4 +1,4 @@ -#' @note To learn more about functions start with `help(package = "gDRstyle")` +#' @note To learn more about functions start with `help(package = "gDRstyle")` #' @keywords internal #' @return package help page "_PACKAGE" diff --git a/R/linter.R b/R/linter.R index 1a7bf31..e57dd5e 100644 --- a/R/linter.R +++ b/R/linter.R @@ -10,13 +10,13 @@ #' pkg_dir= system.file(package = "gDRstyle", "tst_pkgs", "dummy_pkg")) #' #' @return \code{NULL} invisibly. -#' @details +#' @details #' Will look for files in the following directories: #' \code{"R"}, \code{"tests"}, and conditionally \code{"inst/shiny"} #' if \code{shiny} is \code{TRUE}. #' @keywords linter #' @export -lintPkgDirs <- function(pkg_dir = ".", +lintPkgDirs <- function(pkg_dir = ".", shiny = FALSE) { dirs <- c("R", "tests") if (shiny) { @@ -31,7 +31,7 @@ lintPkgDirs <- function(pkg_dir = ".", if (!is.null(failures)) { stop(sprintf( "Found linter failures in files: '%s'", - paste0(failures, collapse = ", ") + paste(failures, collapse = ", ") )) } else { message("All files OK!") @@ -42,7 +42,7 @@ lintPkgDirs <- function(pkg_dir = ".", #' @importFrom lintr lint #' @keywords internal -lintDir <- function(pkg_dir = ".", +lintDir <- function(pkg_dir = ".", sub_dir) { path <- file.path(pkg_dir, sub_dir) if (dir.exists(path)) { diff --git a/R/linter_custom.R b/R/linter_custom.R index 4b76da6..735111a 100644 --- a/R/linter_custom.R +++ b/R/linter_custom.R @@ -1,18 +1,18 @@ #' roxygen_tag_linter -#' +#' #' Check that function has documented specific tag in Roxygen #' skeleton (default \code{@author}). -#' +#' #' @param tag character (default \code{@author}) -#' +#' #' @author Kamil Foltynski -#' +#' #' @examples #' linters_config <- lintr::linters_with_defaults( #' line_length_linter = lintr::line_length_linter(120), #' roxygen_tag_linter = roxygen_tag_linter() #' ) -#' +#' #' @return linter class function #' @keywords linter #' @export @@ -70,7 +70,7 @@ roxygen_tag_linter <- function(tag = "@author") { #' @noRd skip_lines_withou_prefix <- function(flines) { idx <- NA - for (i in seq_len(length(flines))) { + for (i in seq_along(flines)) { if (grepl(pattern = "@noRd", flines[i])) # skip check if @noRd tag is present return(NA) diff --git a/R/linters_config.R b/R/linters_config.R index 62bea34..b4dd221 100644 --- a/R/linters_config.R +++ b/R/linters_config.R @@ -1,3 +1,12 @@ +gDR_undesirable_operators <- + lintr::modify_defaults( + defaults = lintr::default_undesirable_operators, + "<<-" = NULL, + ":::" = NULL, + "%>%" = "please use base R syntax instead of magrittr pipe", + "|>" = "please use base R syntax instead of native pipe" + ) + gDR_undesirable_functions <- lintr::modify_defaults( defaults = lintr::default_undesirable_functions, @@ -11,38 +20,28 @@ gDR_undesirable_functions <- # we prefer not to use these functions because we use data.table as the primary data format "assert_data_frame" = "please use `checkmate::assert_data_table` instead (data.table is primary data format)", "rbind.fill" = "please use `data.table::rbindlist` instead (data.table is primary data format)", - "read.csv" = "pleae use `data.table::fread` instead (data.table is primary data format)", + "read.csv" = "please use `data.table::fread` instead (data.table is primary data format)", "as.data.frame" = "please use `data.table::as.data.table` instead (data.table is primary data format)", "reshape2" = "please use functions from `data.table` package (data.table is primary data format)", - "debug" = NULL + "debug" = NULL, + "nrow" = "please use `NROW` instead (handles NULL and vectors safely)", + "ncol" = "please use `NCOL` instead (handles NULL and vectors safely)" ) #' @noRd linters_config <- - if (packageVersion("lintr") < "3.2.0") { - lintr::linters_with_defaults( - cyclocomp_linter = NULL, - indentation_linter = NULL, - line_length_linter = lintr::line_length_linter(120), - object_name_linter = NULL, - seq_linter = NULL, - trailing_blank_lines_linter = NULL, - trailing_whitespace_linter = NULL, - object_usage_linter = NULL, - object_length_linter = NULL, - undesirable_function_linter = lintr::undesirable_function_linter(fun = gDR_undesirable_functions) - ) - } else { - lintr::linters_with_defaults( - return_linter = NULL, - indentation_linter = NULL, - line_length_linter = lintr::line_length_linter(120), - object_name_linter = NULL, - seq_linter = NULL, - trailing_blank_lines_linter = NULL, - trailing_whitespace_linter = NULL, - object_usage_linter = NULL, - object_length_linter = NULL, - undesirable_function_linter = lintr::undesirable_function_linter(fun = gDR_undesirable_functions) - ) - } + lintr::linters_with_defaults( + cyclocomp_linter = NULL, + expect_true_false_linter = lintr::expect_true_false_linter(), + indentation_linter = NULL, + length_test_linter = lintr::length_test_linter(), + line_length_linter = lintr::line_length_linter(120), + object_name_linter = NULL, + object_length_linter = NULL, + object_usage_linter = NULL, + paste_linter = lintr::paste_linter(), + return_linter = NULL, + undesirable_function_linter = lintr::undesirable_function_linter(fun = gDR_undesirable_functions), + undesirable_operator_linter = lintr::undesirable_operator_linter(op = gDR_undesirable_operators), + yoda_test_linter = lintr::yoda_test_linter() + ) diff --git a/R/news_linter.R b/R/news_linter.R new file mode 100644 index 0000000..7ef19e3 --- /dev/null +++ b/R/news_linter.R @@ -0,0 +1,176 @@ +BANNED_PHRASES <- c( + "in order to", + "it is now possible", + "it is possible", + "has been", + "have been", + "was added", + "were added", + "is now", + "are now", + "functionality", + "as well as" +) + +VALID_VERBS <- c( + "add", "fix", "remove", "update", "extend", "drop", "replace", + "rename", "improve", "change", "bump", "refactor", "move", "extract", + "migrate", "support", "enable", "disable", "allow", "prevent", "ensure", + "expose", "document", "implement", "introduce", "switch", "use", + "deprecate", "restore", "simplify", "unify", "resolve", "enforce", + "include", "exclude", "reduce", "increase", "adjust", "handle" +) + +VERSION_HEADER_PATTERN <- "^## [A-Za-z0-9.]+ \\d+\\.\\d+\\.\\d+ - \\d{4}-\\d{2}-\\d{2}$" + +#' Lint NEWS.md entries for style and brevity +#' +#' Checks that every bullet entry in \code{NEWS.md} follows the gDR style +#' guidelines: starts with an imperative verb, is concise (no verbose phrases, +#' no trailing period, within the character limit), that each version section +#' contains at most \code{max_bullets} entries, and that version headers +#' match the expected format. +#' +#' @param pkg_dir character(1) path to the package root directory containing +#' \code{NEWS.md}. Defaults to the current directory. +#' @param max_chars integer(1) maximum number of characters allowed per bullet +#' entry (excluding the leading \code{"* "}). Defaults to \code{120L}. +#' @param max_bullets integer(1) maximum number of bullet entries allowed per +#' version section. Defaults to \code{3L}. +#' +#' @return \code{NULL} invisibly if no violations are found. Stops with an +#' error listing all violations otherwise. +#' +#' @examples +#' pkg_dir <- system.file(package = "gDRstyle", "tst_pkgs", "dummy_pkg") +#' lintNewsEntries(pkg_dir) +#' +#' @keywords linter +#' @export +lintNewsEntries <- function(pkg_dir = ".", max_chars = 120L, max_bullets = 3L) { + checkmate::assert_directory_exists(pkg_dir) + checkmate::assert_integerish(max_chars, lower = 1L, len = 1L) + checkmate::assert_integerish(max_bullets, lower = 1L, len = 1L) + + news_path <- file.path(pkg_dir, "NEWS.md") + if (!file.exists(news_path)) { + message("No NEWS.md found — skipping news lint.") + return(invisible(NULL)) + } + + lines <- readLines(news_path, warn = FALSE) + violations <- .check_news_lines(lines, max_chars, max_bullets) + + if (length(violations) > 0L) { + stop( + "NEWS.md lint violations:\n", + paste0(" line ", vapply(violations, `[[`, "", "line"), ": ", + vapply(violations, `[[`, "", "msg"), + collapse = "\n") + ) + } + + message("NEWS.md OK!") + invisible(NULL) +} + +#' @keywords internal +.check_news_lines <- function(lines, max_chars, max_bullets) { + violations <- list() + current_header_line <- NULL + bullet_count <- 0L + + flush_section <- function() { + if (!is.null(current_header_line) && bullet_count > max_bullets) { + violations <<- c(violations, list(list( + line = current_header_line, + msg = sprintf( + "section has %d bullets (max %d) — split into separate releases", + bullet_count, max_bullets + ) + ))) + } + } + + for (i in seq_along(lines)) { + line <- lines[[i]] + + if (grepl("^## ", line)) { + flush_section() + current_header_line <- i + bullet_count <- 0L + v <- .check_header(line, i) + if (!is.null(v)) violations <- c(violations, list(v)) + next + } + + if (grepl("^\\* ", line)) { + bullet_count <- bullet_count + 1L + entry <- sub("^\\* ", "", line) + vs <- .check_bullet(entry, i, max_chars) + violations <- c(violations, vs) + } + } + + flush_section() + violations +} + +#' @keywords internal +.check_header <- function(line, line_num) { + if (!grepl(VERSION_HEADER_PATTERN, line)) { + list(line = line_num, msg = sprintf( + "malformed version header (expected '## PkgName X.Y.Z - YYYY-MM-DD'): '%s'", + line + )) + } +} + +#' @keywords internal +.check_bullet <- function(entry, line_num, max_chars) { + violations <- list() + + if (nchar(entry) > max_chars) { + violations <- c(violations, list(list( + line = line_num, + msg = sprintf("entry too long (%d chars, max %d): '%s'", + nchar(entry), max_chars, entry) + ))) + } + + if (grepl("\\.$", entry)) { + violations <- c(violations, list(list( + line = line_num, + msg = sprintf("entry ends with a period: '%s'", entry) + ))) + } + + first_word <- tolower(sub("^([A-Za-z]+).*$", "\\1", entry)) + if (!first_word %in% VALID_VERBS) { + violations <- c(violations, list(list( + line = line_num, + msg = sprintf( + "entry does not start with a known imperative verb (got '%s'): '%s'", + first_word, entry + ) + ))) + } + + for (phrase in BANNED_PHRASES) { + if (grepl(phrase, entry, ignore.case = TRUE)) { + violations <- c(violations, list(list( + line = line_num, + msg = sprintf("entry contains banned phrase '%s': '%s'", phrase, entry) + ))) + } + } + + if (grepl("\\bGDR-[0-9]+\\b", entry)) { + violations <- c(violations, list(list( + line = line_num, + msg = sprintf("entry contains Jira ticket reference: '%s'", entry) + ))) + } + + violations +} diff --git a/inst/scripts/run_tests.R b/inst/scripts/run_tests.R index 8f5de98..86677b4 100644 --- a/inst/scripts/run_tests.R +++ b/inst/scripts/run_tests.R @@ -13,19 +13,19 @@ CHECK_VIGNETTES <- as.logical(args[8]) stopifnot(dir.exists(LIB_DIR)) invisible( sapply( - list.files(LIB_DIR, , pattern = "*.R$", full.names = TRUE), - source, + list.files(LIB_DIR, , pattern = "*.R$", full.names = TRUE), + source, .GlobalEnv ) ) # Check package gDRstyle::checkPackage( - pkgName = PKG_NAME, - repoDir = REPO_DIR, - subdir = PKG_SUBDIR, - fail_on = FAIL_ON, - bioc_check = BIOC_CHECK, + pkgName = PKG_NAME, + repoDir = REPO_DIR, + subdir = PKG_SUBDIR, + fail_on = FAIL_ON, + bioc_check = BIOC_CHECK, run_examples = RUN_EXAMPLES, build_vignettes = CHECK_VIGNETTES, check_vignettes = CHECK_VIGNETTES diff --git a/inst/tst_pkgs/dummy_pkg/NEWS.md b/inst/tst_pkgs/dummy_pkg/NEWS.md new file mode 100644 index 0000000..ea603ab --- /dev/null +++ b/inst/tst_pkgs/dummy_pkg/NEWS.md @@ -0,0 +1,2 @@ +## fakePkg 1.0.0 - 2019-12-17 +* Add initial implementation diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R index 288eaea..37f3b30 100644 --- a/tests/testthat/test-build.R +++ b/tests/testthat/test-build.R @@ -14,7 +14,7 @@ testthat::test_that("set tokens properly", { Sys.unsetenv("DUMMY_ENV") Sys.unsetenv("DUMMY_ENV_2") }) - + base_dir <- system.file(package = "gDRstyle", "tst_tokens") # SINGLE TOKEN setTokenVar(base_dir, "dummy_single_token.txt") diff --git a/tests/testthat/test-linters_config.R b/tests/testthat/test-linters_config.R index 0a35423..b9dcc33 100644 --- a/tests/testthat/test-linters_config.R +++ b/tests/testthat/test-linters_config.R @@ -7,4 +7,4 @@ testthat::test_that("custom `gDR_undesirable_functions` works as expected", { lint_results_df <- data.frame(lint_results, stringsAsFactors = FALSE) testthat::expect_equal(lint_results_df$line_number, c(7L, 19L)) testthat::expect_true(all(grepl("\"assert_data_frame\"|\"rbind.fill\"", lint_results_df$message))) -}) \ No newline at end of file +}) diff --git a/tests/testthat/test-news_linter.R b/tests/testthat/test-news_linter.R new file mode 100644 index 0000000..73e26f4 --- /dev/null +++ b/tests/testthat/test-news_linter.R @@ -0,0 +1,96 @@ +test_that(".check_bullet flags entry that is too long", { + long <- strrep("x", 121L) + vs <- .check_bullet(long, 1L, 120L) + expect_true(any(grepl("too long", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet accepts entry within limit", { + vs <- .check_bullet("Add support for new linter rule", 1L, 120L) + expect_length(vs, 0L) +}) + +test_that(".check_bullet flags trailing period", { + vs <- .check_bullet("Fix broken test.", 1L, 120L) + expect_true(any(grepl("ends with a period", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet flags unknown first word", { + vs <- .check_bullet("New linter added", 1L, 120L) + expect_true(any(grepl("imperative verb", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet flags banned phrase", { + vs <- .check_bullet("Add functionality for parsing", 1L, 120L) + expect_true(any(grepl("banned phrase", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet flags 'has been' passive voice", { + vs <- .check_bullet("Fix issue that has been present since v1", 1L, 120L) + expect_true(any(grepl("has been", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet flags Jira ticket reference", { + vs <- .check_bullet("Fix parsing bug GDR-1234", 1L, 120L) + expect_true(any(grepl("Jira ticket", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_bullet accepts entry without Jira reference", { + vs <- .check_bullet("Fix parsing bug in annotation", 1L, 120L) + expect_false(any(grepl("Jira ticket", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_news_lines flags section with too many bullets", { + lines <- c( + "## myPkg 1.0.0 - 2026-01-01", + "* Add feature one", + "* Fix bug two", + "* Update dependency three", + "* Remove old code four" + ) + vs <- .check_news_lines(lines, 120L, 3L) + expect_true(any(grepl("4 bullets", vapply(vs, `[[`, "", "msg")))) +}) + +test_that(".check_news_lines accepts section within bullet limit", { + lines <- c( + "## myPkg 1.0.0 - 2026-01-01", + "* Add feature one", + "* Fix bug two", + "* Update dependency three" + ) + vs <- .check_news_lines(lines, 120L, 3L) + expect_length(vs, 0L) +}) + +test_that(".check_header accepts valid version header", { + expect_null(.check_header("## gDRstyle 1.2.3 - 2026-01-15", 1L)) +}) + +test_that(".check_header flags malformed header", { + v <- .check_header("## gDRstyle v1.2.3", 1L) + expect_true(grepl("malformed", v$msg)) +}) + +test_that("lintNewsEntries passes on valid NEWS.md", { + pkg_dir <- withr::local_tempdir() + writeLines(c( + "## myPkg 1.0.0 - 2026-01-01", + "* Add initial implementation", + "* Fix edge case in parser" + ), file.path(pkg_dir, "NEWS.md")) + expect_message(lintNewsEntries(pkg_dir), "OK") +}) + +test_that("lintNewsEntries stops on violations", { + pkg_dir <- withr::local_tempdir() + writeLines(c( + "## myPkg 1.0.0 - 2026-01-01", + "* This functionality has been added." + ), file.path(pkg_dir, "NEWS.md")) + expect_error(lintNewsEntries(pkg_dir), "violations") +}) + +test_that("lintNewsEntries skips missing NEWS.md with message", { + pkg_dir <- withr::local_tempdir() + expect_message(lintNewsEntries(pkg_dir), "No NEWS.md") +}) diff --git a/vignettes/gDRstyle.Rmd b/vignettes/gDRstyle.Rmd index a82dfaf..00dae50 100644 --- a/vignettes/gDRstyle.Rmd +++ b/vignettes/gDRstyle.Rmd @@ -4,7 +4,7 @@ author: - name: gDR team email: gdr-support-d@gene.com package: gDRstyle -date: +date: output: BiocStyle::html_document: toc_float: yes @@ -21,28 +21,28 @@ library(gDRstyle) # Overview -The `gDRstyle` package is intended to be used during development of packages -within the gDR platform. It has 3 primary uses: (1)to set a style guide with -functions that check that the style is upheld, (2) during CI to ensure code -passes `R CMD check` to maintain the state of the code in high quality, and (3) +The `gDRstyle` package is intended to be used during development of packages +within the gDR platform. It has 3 primary uses: (1)to set a style guide with +functions that check that the style is upheld, (2) during CI to ensure code +passes `R CMD check` to maintain the state of the code in high quality, and (3) for package dependency installation during gDR platform image building. # Use Cases ## Style guide -See the written [Style guide](https://gdrplatform.github.io/gDRstyle/articles/style_guide.html). -The function `lintPkgDirs` can be used to ensure the package is appropriately -linted. +See the written [Style guide](https://gdrplatform.github.io/gDRstyle/articles/style_guide.html). +The function `lintPkgDirs` can be used to ensure the package is appropriately +linted. ## CI/CD -The `checkPackage` function will check that the package abides by gDRstyle -stylistic requirements, passes `rcmdcheck`, and ensures that the -`dependencies.yml` file used to build gDR platform's docker image is kept -up-to-date with the dependencies listed in the package's `DESCRIPTION` file. -This is called in gDR platform packages' CI/CD. +The `checkPackage` function will check that the package abides by gDRstyle +stylistic requirements, passes `rcmdcheck`, and ensures that the +`dependencies.yml` file used to build gDR platform's docker image is kept +up-to-date with the dependencies listed in the package's `DESCRIPTION` file. +This is called in gDR platform packages' CI/CD. ## Package installation -The function `installAllDeps` assists in installing package dependencies. +The function `installAllDeps` assists in installing package dependencies. For example, it's used in gdrplatform packages (see e.g. [link](https://github.com/gdrplatform/gDR/blob/main/Dockerfile)). # SessionInfo {-} diff --git a/vignettes/style_guide.Rmd b/vignettes/style_guide.Rmd index 781062e..729ea10 100644 --- a/vignettes/style_guide.Rmd +++ b/vignettes/style_guide.Rmd @@ -4,7 +4,7 @@ author: - name: gDR team email: gdr-support-d@gene.com package: gDRstyle -date: +date: output: BiocStyle::html_document: toc_float: yes @@ -66,7 +66,7 @@ fun <- function(param1, param2, param_with_dir_for_st_important = file.path(syst * All internal functions are not exported * All exported functions have `assert` tests for their parameters * `vapply` over `sapply` (or `lapply` + `unlist()` if predefining FUN.VALUE is difficult) - * Do not use `apply` on data.frame(s) (`mapply` is good for row-wise operations) + * Do not use `apply` on data.frame(s) (`mapply` is good for row-wise operations) * Function returns * Use implicit returns over explicit @@ -114,7 +114,30 @@ what_is_going_on <- if (is_check()) { + this includes: `checkmate`, `SummarizedExperiment`, etc. 10. Tests * `expect_equal(obs, exp)` over `expect_equal(exp, obs)` -11. Miscellaneous + * Use specific `testthat` expectations over generic ones: + * `expect_true(x == y)` → `expect_equal(x, y)` + * `expect_true(x)` / `expect_false(x)` → `expect_true()`/`expect_false()` (not `expect_equal(x, TRUE)`) + * `expect_equal(length(x), n)` → `expect_length(x, n)` + * Do not leave commented-out code in test files. +11. Pipes + * Do not use pipe operators (`%>%` or `|>`); use base R syntax instead. +12. String concatenation + * Use `paste0()` over `paste(..., sep = "")`. +13. Sequence generation + * Use `seq_len(n)` over `1:n` and `seq_along(x)` over `1:length(x)` to avoid off-by-one errors when `n` is zero. +14. Boolean values + * Use `TRUE`/`FALSE` instead of `T`/`F`. `T` and `F` are regular variables that can be overwritten (`T <- 5`). +15. NA comparisons + * Use `is.na(x)` instead of `x == NA`. The expression `x == NA` always returns `NA`, never `TRUE` or `FALSE`. +16. Logical operators + * Use `&&` and `||` in scalar conditions (e.g. inside `if`). Use `&` and `|` for element-wise operations on vectors. +17. Strings + * Use double quotes `"` instead of single quotes `'`. +18. Commented-out code + * Do not leave commented-out code in the codebase. Use version control instead. +19. Function complexity + * Keep cyclomatic complexity below 25. If a function exceeds this limit, extract logic into smaller helper functions. +20. Miscellaneous * Exponentiation: always utilize `^` over `**` for exponentiation like `2 ^ 3` over `2**3`. * Numerics: place `0`'s in front of decimals like `0.1` over `.1` * Use named indexing over positional indexing `df[, "alias"] <- df[, "celllinename"]` over `df[, 1] <- df[, 2]` @@ -144,7 +167,7 @@ if (length(foo()) == 1) { 1. Create file _{pkgname}-package.R_ with `usethis::use_package_doc()` 2. Update _{pkgname}-package.R_ - it should have such lines: ```{r} -#' @note To learn more about functions start with `help(package = "{pkgname}")` +#' @note To learn more about functions start with `help(package = "{pkgname}")` #' @keywords internal #' @return package help page "_PACKAGE" @@ -159,18 +182,20 @@ if the requirements for Bioconductor are also met) ## Git best practices 1. Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) * Commit messages should look like `: ` where `type` can be one of: - * `fix`: for bugfixes; + * `fix`: for bugfixes; * `feat`: for new features; * `docs`: for documentation changes; * `style`: for formatting changes that do not affect the meaning of the code; * `test`: for adding missing tests or correcting existing tests; * `refactor`: for code changes that neither fixes a bug nor adds a feature * `ci`: for changes to CI configuration + * `chore`: for version bumps, NEWS.md updates, and build tooling changes 2. Any change in code should be accompanied by a bumped version. - * new features - `PATCH` version; - * bugfixes and other changes - `MINOR` version. -*Exceptions*: All public packages - as to-be-released on Bioconductor have version 0.99.x. + * new features - `MINOR` version; + * bugfixes and other changes - `PATCH` version. +*Exceptions*: All public packages - as to-be-released on Bioconductor have version 0.99.x. Any changes in code should be accompanied by a bumped `MINOR` version regardless of the nature of the changes. +3. If a PR does not include a version bump in `DESCRIPTION` and a matching entry in `NEWS.md`, the `auto-changelog` CI workflow will generate and commit them automatically using the PR diff. # SessionInfo {-}