Skip to content

Conversation

@gnoblet
Copy link
Owner

@gnoblet gnoblet commented Dec 31, 2025

No description provided.

Copilot AI review requested due to automatic review settings January 1, 2026 15:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request migrates the project's R package management from renv to rv, a newer package manager. The change includes new activation scripts, configuration files, and updates to the GitHub Actions workflow to support the rv-based dependency management system.

Key Changes

  • Replaced renv with rv package manager for R dependency management
  • Added rv wrapper functions and activation scripts for seamless integration
  • Updated CI/CD pipeline to install and configure rv in GitHub Actions
  • Migrated project configuration from renv.lock to rproject.toml format

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
rv/scripts/rvr.R Wrapper functions providing R interface to rv CLI commands (summary, plan, sync, add)
rv/scripts/activate.R Activation script that configures R library paths and repositories based on rv configuration
rv/.gitignore Ignores the rv library directory from version control
rv.lock Auto-generated lock file containing pinned package versions and dependencies
rproject.toml New rv configuration file defining project dependencies and R version
_quarto.yml Updated to exclude rv directory from Quarto rendering
.github/workflows/quarto-publish.yml Updated workflow to install rv, restore packages, and configure library paths
.Rprofile Replaced renv activation with rv activation scripts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +107 to +108
echo "R_LIBS_USER=$HOME/rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV
echo "R_LIBS_SITE=$HOME/rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The library path configuration is inconsistent between the step environment variable definition and the later override. Line 24 sets R_LIBS_USER to "$HOME/.rv/library:$HOME/.local/lib/R/site-library", but line 107 sets it to "$HOME/rv/library:$HOME/.local/lib/R/site-library" (missing the dot before "rv"). This inconsistency will cause the R library path to point to different directories, potentially causing package resolution failures.

Suggested change
echo "R_LIBS_USER=$HOME/rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV
echo "R_LIBS_SITE=$HOME/rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV
echo "R_LIBS_USER=$HOME/.rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV
echo "R_LIBS_SITE=$HOME/.rv/library:$HOME/.local/lib/R/site-library" >> $GITHUB_ENV

Copilot uses AI. Check for mistakes.
Comment on lines 110 to 114
# - name: Build Quarto website
run: |
# export R_LIBS_USER="$HOME/.rv/library:$HOME/.local/lib/R/site-library"
# export R_LIBS_SITE="$HOME/.rv/library:$HOME/.local/lib/R/site-library"
quarto render
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The "Build Quarto website" step name is commented out but the actual run command is not commented. This creates a step without a name, which will make the GitHub Actions logs less clear and harder to debug. Either uncomment the name or add a new appropriate name for this step.

Copilot uses AI. Check for mistakes.
Comment on lines +118 to +140
# - name: Check R library paths
# run: Rscript -e '.libPaths()'

# # 3️⃣ Display the R_LIBS_USER variable
# - name: Print R_LIBS_USER
# run: |
# echo "R_HOME = $R_HOME"
# echo "R_LIBS_USER = $R_LIBS_USER"
# echo "R_LIBS_SITE = $R_LIBS_SITE"

# - name: quarto check
# run: |
# quarto check

# - name: Print repository tree
# run: |
# echo "Repository tree (depth=3):"
# if command -v tree >/dev/null 2>&1; then
# tree -a -L 3
# else
# find . -maxdepth 3 -print
# fi

Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

Large blocks of commented-out debugging code should be removed before merging. These commented sections (lines 112-113, 117-139) appear to be temporary debugging steps that are no longer needed. Keeping them reduces code readability and makes the workflow harder to maintain. If these are needed for future debugging, consider documenting the approach in a separate file or issue.

Suggested change
# - name: Check R library paths
# run: Rscript -e '.libPaths()'
# # 3️⃣ Display the R_LIBS_USER variable
# - name: Print R_LIBS_USER
# run: |
# echo "R_HOME = $R_HOME"
# echo "R_LIBS_USER = $R_LIBS_USER"
# echo "R_LIBS_SITE = $R_LIBS_SITE"
# - name: quarto check
# run: |
# quarto check
# - name: Print repository tree
# run: |
# echo "Repository tree (depth=3):"
# if command -v tree >/dev/null 2>&1; then
# tree -a -L 3
# else
# find . -maxdepth 3 -print
# fi

Copilot uses AI. Check for mistakes.
args <- c(command, "-c", .rv$config_path)
res <- system2("rv", args, stdout = TRUE)
if (!is.null(attr(res, "status"))) {
warning(sprintf("failed to run `rv %s`, check your console for messages", paste(args, collapse = " ")))
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The error message references an incorrect command format. The warning message shows "rv %s" but the actual command being executed uses "rv" followed by the arguments. The sprintf format should be "rv %s" with paste(command, collapse = " ") to show the actual command that failed, not "rv %s" with paste(args, collapse = " ") which includes the "-c" flag and config path that are added by the wrapper.

Suggested change
warning(sprintf("failed to run `rv %s`, check your console for messages", paste(args, collapse = " ")))
warning(sprintf("failed to run `rv %s`, check your console for messages", paste(command, collapse = " ")))

Copilot uses AI. Check for mistakes.
# ---- rv: install and restore project library ----
- name: Install rv (A2-ai)
run: |
curl -sSL https://raw.githubusercontent.com/A2-ai/rv/refs/heads/main/scripts/install.sh | bash
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The installation script is sourced directly from the main branch without version pinning or checksum validation. This could lead to unexpected breaking changes if the installation script is updated. Consider pinning to a specific commit SHA or version tag for reproducibility and stability in the CI environment.

Suggested change
curl -sSL https://raw.githubusercontent.com/A2-ai/rv/refs/heads/main/scripts/install.sh | bash
curl -sSL https://raw.githubusercontent.com/A2-ai/rv/refs/tags/v0.1.0/scripts/install.sh | bash

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +52
"WARNING: R version specified in config (%s) does not match session version (%s).
rv library will not be activated until the issue is resolved. Entering safe mode...
",
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The warning message uses inconsistent indentation and formatting. The multi-line string on lines 50-52 has irregular whitespace that will appear awkwardly in the console output. The message should use consistent indentation or be reformatted as a single line with proper formatting.

Suggested change
"WARNING: R version specified in config (%s) does not match session version (%s).
rv library will not be activated until the issue is resolved. Entering safe mode...
",
paste0(
"WARNING: R version specified in config (%s) does not match session version (%s).\n",
"rv library will not be activated until the issue is resolved. Entering safe mode...\n"
),

Copilot uses AI. Check for mistakes.
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.

2 participants