Document cpflow upstream ref pinning#743
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR adds and refines documentation across four files to explain how generated GitHub Actions ChangesUpstream Pinning & CPFLOW_VERSION Guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds documentation clarifying how generated
Confidence Score: 5/5Documentation-only changes with no runtime code modified; safe to merge. All four changed files are Markdown documentation. The new content is accurate, internally consistent across files, and aligns with how GitHub Actions reusable workflows actually resolve refs. The transient note about the current commit-SHA pin is an honest description of the repository's state and does not introduce any risk. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Need to update cpflow workflows] --> B{Testing unreleased\nupstream changes?}
B -- No --> C[Install released cpflow gem]
C --> D[Run cpflow generate-github-actions\n--staging-branch master]
D --> E[Generated wrappers set\nuses: ...@v<gem version>\ncontrol_plane_flow_ref: v<gem version>]
E --> F{Set CPFLOW_VERSION?}
F -- Leave unset --> G[Setup action builds cpflow\nfrom checked-out upstream ref]
F -- Set to gem version\nwithout leading v --> H[Setup action runs\ngem install cpflow -v <version>]
B -- Yes --> I[Pin to immutable commit SHA\nnot a branch name]
I --> J[Manually set uses: ...@<SHA>\nand control_plane_flow_ref: <SHA>]
J --> K[Temporary test pin]
K --> L{Upstream release published?}
L -- Yes --> C
L -- No --> M[Keep SHA pin until\nrelease tag is available]
Reviews (1): Last reviewed commit: "Document cpflow upstream ref pinning" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d74319f988
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| a Ruby gem; GitHub resolves them from a repository ref. Each wrapper therefore | ||
| has two upstream pins that must stay in sync: |
There was a problem hiding this comment.
Qualify claim that every wrapper has two upstream pins
This states that each generated cpflow-* wrapper has both uses: ...@<ref> and control_plane_flow_ref, but that is not true for all generated wrappers in this repo (for example, .github/workflows/cpflow-review-app-help.yml and .github/workflows/cpflow-help-command.yml only pin the uses ref). Following this wording when repinning could lead maintainers to add an unsupported with.control_plane_flow_ref input to workflows that do not accept it, causing workflow-call validation failures.
Useful? React with 👍 / 👎.
Code Review: Document cpflow upstream ref pinningOverview Documentation-only PR (70 additions, 2 deletions across 4 Markdown files) clarifying:
Strengths
Issues and Suggestions1. Ephemeral state statement will become stale (see inline comment) The last paragraph of the new section in 2. Missing consequence of version mismatch The docs say the two refs must "stay in sync" but do not explain what breaks if they diverge. A one-sentence note such as: If the two refs diverge, the composite actions loaded at runtime may not match the reusable workflow logic, producing unpredictable deployment behavior. would help readers understand the severity. 3. Minor: The phrase "use the RubyGems version without a leading 4. Readme section placement In SummaryLow-risk, well-written docs PR. The core content is accurate and fills a real gap. The main actionable item is the ephemeral "currently pinned" paragraph — it should be removed or marked as a TODO comment before merge to avoid becoming misleading once the upstream release lands. |
| workflow change was tested before a new `cpflow` gem release existed. That is | ||
| safer than pinning a branch, but it should be treated as a temporary test pin | ||
| until the next upstream release tag is available. | ||
|
|
There was a problem hiding this comment.
This paragraph describes a transient repo state ("currently pinned to an upstream commit SHA") that will become factually wrong once the upstream release is published and the pin is updated to a release tag.
Recommend either removing it entirely (the general guidance above it is sufficient) or converting to an HTML comment so it is hidden from rendered docs but easy to find and delete later:
| This repo is currently pinned to an upstream commit SHA because the reusable | |
| workflow change was tested before a new `cpflow` gem release existed. That is | |
| safer than pinning a branch, but it should be treated as a temporary test pin | |
| until the next upstream release tag is available. | |
❌ Failed to Delete Review AppFailed to delete review app for PR #743 |
Summary
Verification
Note
Low Risk
Documentation-only changes clarifying how generated
cpflow-*workflows pin upstream refs and howCPFLOW_VERSIONaffects gem installation; no runtime code or CI behavior changes.Overview
Documents how generated
cpflow-*GitHub Actions pin upstreamshakacode/control-plane-flowcode. Adds an explicit explanation that each wrapper must keepuses: ...@<ref>andcontrol_plane_flow_ref: <ref>in sync, and recommends stable pinning tov<cpflow gem version>tags (using commit SHAs only for temporary unreleased testing).Clarifies that
CPFLOW_VERSIONis a runtime gem-install override distinct from the GitHub workflow ref (build-from-upstream when unset vsgem install -vwhen set), and updates team/help docs to reflect this guidance.Reviewed by Cursor Bugbot for commit d74319f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
CPFLOW_VERSIONcontrols the runtime gem installation.