Skip to content

Stamp dlw#17

Closed
dana89co wants to merge 3 commits intomasterfrom
stamp_dlw
Closed

Stamp dlw#17
dana89co wants to merge 3 commits intomasterfrom
stamp_dlw

Conversation

@dana89co
Copy link
Copy Markdown
Contributor

@dana89co dana89co commented Mar 30, 2026

Hola @randrescastaneda,

I wanted to add stamp to the metapip packages, but I thought to add as well to add dlw. Also, since now we are using PROD as the main branch (Tefera already merged the last changes to this branch), then I made it the default branch. If you do not agree to any of these changes, I can change it accordingly. Thank you for reviewing.

Summary by CodeRabbit

  • New Features

    • Core managed packages expanded to include dlw and stamp alongside existing pip-* packages.
    • Added per-package branch settings for dlw and stamp (both set to main).
  • Changes

    • Default branch value changed from DEV_v2 to PROD.
  • Documentation

    • README updated to reflect new core packages and revised default-branch examples.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

📝 Walkthrough

Walkthrough

The core package list managed by metapip now includes dlw and stamp. The default branch option changed from "DEV_v2" to "PROD", and per-package branch overrides for dlw and stamp were added. Documentation updated to reflect these changes.

Changes

Cohort / File(s) Summary
Core Package Management
R/attach.R
Expanded the core variable to add "dlw" and "stamp", altering which packages are considered by pkg_loaded(), pkg_unloaded(), and metapip_attach().
Configuration & Defaults
R/zzz.R
Changed options(metapip.default_branch) from "DEV_v2" to "PROD"; added stamp_branch = "main" and dlw_branch = "main" keys to metapip.custom_branch.
Documentation
README.Rmd, README.md
Updated "Core packages" list (removed pipr, added pipster, pipfaker, dlw, stamp) and revised examples/text to reflect default branch change to "PROD" and new per-package branch assignments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Two new names hop into core,
dlw and stamp — we add them more.
The default branch now says "PROD",
New mappings set, configurations nod.
A tiny hop, a tidy change, hooray!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Stamp dlw' is vague and does not clearly convey what changes were made. It only mentions package names without explaining the action taken or the significance of the change. Consider a more descriptive title such as 'Add stamp and dlw packages to core' or 'Include stamp and dlw in core packages and update default branch' to clearly communicate the main changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stamp_dlw

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@R/zzz.R`:
- Line 18: Update the documentation that references the default branch to match
the code change: replace occurrences of the old default branch name ("DEV_v2")
with the new value ("PROD") in the README.Rmd paragraph that documents
metapip.default_branch so the README text reflects the current default set in
metapip.default_branch.
- Line 18: The project-wide default metapip.default_branch is set to "PROD" but
several packages (pipload, pipfun, pipdata, pipaux) do not have a PROD branch,
causing get_current_branches() and package_branches() to fail; fix by either
creating a PROD branch in each of those repos or changing metapip.default_branch
to a branch that all packages actually have (e.g., main/master) and update the
value in zzz.R (metapip.default_branch) so it matches the existing branch across
pipload, pipfun, pipdata, and pipaux; ensure no other code assumes PROD
elsewhere (search for get_current_branches and package_branches usage) and
adjust accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c75fbcc9-ad34-45f5-b040-e4a7900b6e6b

📥 Commits

Reviewing files that changed from the base of the PR and between 81c06da and d215aaa.

📒 Files selected for processing (3)
  • R/attach.R
  • R/zzz.R
  • README.Rmd


metapip_default_options <- list(
metapip.default_branch = "DEV_v2",
metapip.default_branch = "PROD",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Default-branch docs are stale after this change.

metapip.default_branch now defaults to "PROD", but README still says "DEV_v2" (README.Rmd Line 54 and the same paragraph). Please update that section to avoid misleading setup guidance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@R/zzz.R` at line 18, Update the documentation that references the default
branch to match the code change: replace occurrences of the old default branch
name ("DEV_v2") with the new value ("PROD") in the README.Rmd paragraph that
documents metapip.default_branch so the README text reflects the current default
set in metapip.default_branch.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

org="PIP-Technical-Team"
# These inherit metapip.default_branch = "PROD" based on current overrides.
repos=(pipload pipfun pipdata pipaux)

for repo in "${repos[@]}"; do
  if gh api "repos/${org}/${repo}/branches/PROD" --jq '.name' >/dev/null 2>&1; then
    echo "OK: ${repo} has PROD"
  else
    echo "MISSING: ${repo} does not have PROD"
  fi
done

Repository: PIP-Technical-Team/metapip

Length of output: 211


All packages that inherit metapip.default_branch = "PROD" lack a PROD branch.

Packages pipload, pipfun, pipdata, and pipaux do not have a PROD branch, which will cause runtime failures in get_current_branches() and package_branches(). Either create the PROD branch across all packages or change the default to a branch that exists on all of them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@R/zzz.R` at line 18, The project-wide default metapip.default_branch is set
to "PROD" but several packages (pipload, pipfun, pipdata, pipaux) do not have a
PROD branch, causing get_current_branches() and package_branches() to fail; fix
by either creating a PROD branch in each of those repos or changing
metapip.default_branch to a branch that all packages actually have (e.g.,
main/master) and update the value in zzz.R (metapip.default_branch) so it
matches the existing branch across pipload, pipfun, pipdata, and pipaux; ensure
no other code assumes PROD elsewhere (search for get_current_branches and
package_branches usage) and adjust accordingly.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
README.md (1)

62-64: Clarify scope of the branch-behavior example.

Line 62–64 can be misread as if the shown options(...) call alone sets wbpip, pipster, stamp, and dlw. Consider explicitly saying these come from package defaults unless overridden.

Suggested doc tweak
-So in this case, the default value for `pipapi`, `wbpip` and `pipster`
-branch is “DEV” whereas the one for `pipfaker`,`stamp` and `dlw` is set
-to “main”. For rest of the pip core packages it remains as “PROD”.
+With the package defaults, `pipapi`, `wbpip`, and `pipster` use “DEV”,
+while `pipfaker`, `stamp`, and `dlw` use “main” (unless overridden in
+`metapip.custom_branch`). The remaining core packages use “PROD”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 62 - 64, Clarify that the example describes how
branch defaults are determined: state explicitly that the shown options(...)
call sets the default branch values for the named entries (pipapi, wbpip,
pipster) to "DEV" and for pipfaker, stamp, dlw to "main", while all other pip
core packages retain their "PROD" default from their individual package defaults
unless those defaults are overridden; update the text to mention the
options(...) call vs. package defaults and list the specific symbols (pipapi,
wbpip, pipster, pipfaker, stamp, dlw, "DEV", "main", "PROD") so readers cannot
misinterpret which values come from options(...) and which come from package
defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@README.md`:
- Around line 62-64: Clarify that the example describes how branch defaults are
determined: state explicitly that the shown options(...) call sets the default
branch values for the named entries (pipapi, wbpip, pipster) to "DEV" and for
pipfaker, stamp, dlw to "main", while all other pip core packages retain their
"PROD" default from their individual package defaults unless those defaults are
overridden; update the text to mention the options(...) call vs. package
defaults and list the specific symbols (pipapi, wbpip, pipster, pipfaker, stamp,
dlw, "DEV", "main", "PROD") so readers cannot misinterpret which values come
from options(...) and which come from package defaults.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5ecb4cc7-8f67-429a-afa0-7eef0e0481d9

📥 Commits

Reviewing files that changed from the base of the PR and between d215aaa and 08f75b4.

📒 Files selected for processing (2)
  • README.Rmd
  • README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.Rmd

@dana89co
Copy link
Copy Markdown
Contributor Author

Hola @randrescastaneda, never mind, I just realized it is a bit more complicated because the packages (stamp, dlw) are not part of the PIP-Technical-Team. I will try to work a bit more on it before asking you to approve the PR.

@dana89co dana89co closed this Mar 30, 2026
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.

1 participant