Skip to content

Clarify review app help opt-out#323

Merged
justin808 merged 1 commit into
mainfrom
jg-codex/cpflow-release-polish
May 25, 2026
Merged

Clarify review app help opt-out#323
justin808 merged 1 commit into
mainfrom
jg-codex/cpflow-release-polish

Conversation

@justin808

@justin808 justin808 commented May 25, 2026

Copy link
Copy Markdown
Member

Summary

  • clarify generated PR-open help opt-out wording for forks and clones
  • add the suggested guard comment directly to the generated wrapper
  • cover the generated wording in specs

Verification

  • CPLN_ORG=test-org bundle exec rspec spec/command/generate_github_actions_spec.rb
  • RUBOCOP_CACHE_ROOT=/private/tmp/rubocop_cache bundle exec rubocop lib/github_flow_templates/bin/pin-cpflow-github-ref spec/command/generate_github_actions_spec.rb
  • bundle exec rake check_command_docs
  • git diff --check

Note

Low Risk
Documentation and generated template comment changes only; default PR-open help behavior is unchanged.

Overview
Clarifies how downstream repos can opt out of automatic PR-open review-app command comments when they inherit generated workflows before Control Plane is ready.

Generated cpflow-review-app-help.yml comments now explain that committing the wrapper opts in, and point forks/clones at removing the file or enabling a sample job guard (vars.REVIEW_APP_PREFIX != '' || vars.CPLN_ORG_STAGING != ''). The same guidance is mirrored in cpflow-help.md, docs/ci-automation.md, and a CHANGELOG entry. Specs assert the generated wrapper and help markdown include that guard example.

Reviewed by Cursor Bugbot for commit 4b1ca18. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Documentation
    • Enhanced PR-open help workflow documentation clarifying when review app command references are posted
    • Provided guidance for forks/clones to suppress help messages before Control Plane configuration
    • Updated GitHub Actions workflow with optional conditional guard examples

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16675101-09d0-4294-9860-116ffcfe359a

📥 Commits

Reviewing files that changed from the base of the PR and between c0ca750 and 4b1ca18.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • docs/ci-automation.md
  • lib/github_flow_templates/.github/cpflow-help.md
  • lib/github_flow_templates/.github/workflows/cpflow-review-app-help.yml
  • spec/command/generate_github_actions_spec.rb

Walkthrough

This PR clarifies documentation and test expectations for a GitHub Actions workflow guard that controls when PR-open help comments appear. The workflow template includes an intentionally unconditional wrapper with a commented-out conditional example, and tests verify the guard condition is present in generated output. User guides explain the behavior and fork/clone mitigation options.

Changes

PR-open help wrapper guard documentation

Layer / File(s) Summary
Workflow template and generated content expectations
lib/github_flow_templates/.github/workflows/cpflow-review-app-help.yml, spec/command/generate_github_actions_spec.rb
The workflow comment block clarifies the wrapper is intentionally unconditional and includes a commented-out if: vars.REVIEW_APP_PREFIX != '' || vars.CPLN_ORG_STAGING != '' example. Two test assertions verify the condition is present in generated PR-open help wrapper and long-form markdown content.
User-facing documentation updates
CHANGELOG.md, docs/ci-automation.md, lib/github_flow_templates/.github/cpflow-help.md
Changelog entry, CI automation guide, and generated template guidance explain that the PR-open help comment is posted whenever the wrapper exists for configured repos, and that forks/clones can delete the wrapper file or apply the if: guard to suppress help until Control Plane is configured.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • shakacode/control-plane-flow#285: Both PRs modify the cpflow-review-app-help.yml workflow and generator spec expectations; main PR adds a vars.REVIEW_APP_PREFIX || vars.CPLN_ORG_STAGING wrapper guard while retrieved PR focuses on renaming/pinning command strings and triggers.
  • shakacode/control-plane-flow#319: The main PR's updates to cpflow-help.md about PR-open help wrapper behavior and fork handling build on the retrieved PR's rewrite of the same generated documentation.

Suggested labels

review needed

Poem

🐰 A wrapper guard to hush the noise,
Till config blooms, then help deploys,
For forks who copy workflows clean,
A gentle if: keeps silence serene.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Clarify review app help opt-out' directly matches the main change: clarifying opt-out wording and adding guard comments for review app help in generated workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/cpflow-release-polish

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.

@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown

Greptile Summary

This PR clarifies the opt-out path for the generated PR-open review-app help workflow, targeting forks and clones that copy the wrapper before Control Plane is configured. The core change is embedding a concrete, commented-out if: guard directly in the generated YAML template so users have an actionable example without leaving the file.

  • cpflow-review-app-help.yml template gains a commented # if: vars.REVIEW_APP_PREFIX != '' || vars.CPLN_ORG_STAGING != '' line with "uncomment and adapt" guidance, replacing the previous vague "add a wrapper-level if: guard" prose.
  • Both docs/ci-automation.md and lib/github_flow_templates/.github/cpflow-help.md receive the same clarified wording with the concrete guard example, keeping them in sync.
  • Two new spec assertions verify the guard example string is present in both the generated wrapper and the help markdown.

Confidence Score: 5/5

All changes are documentation, template comments, and spec assertions — no runtime logic is altered.

The three wording changes (workflow template comment, CI docs, help markdown) are consistent with each other and reference a syntactically valid GitHub Actions vars context expression. The two new spec assertions directly assert the guard example string appears in generated output, closing the gap between the template and the test suite. No executable code paths are modified.

No files require special attention.

Important Files Changed

Filename Overview
lib/github_flow_templates/.github/workflows/cpflow-review-app-help.yml Comment block updated to include a concrete commented-out if: guard example with correct GitHub Actions vars context syntax.
spec/command/generate_github_actions_spec.rb Two new expect assertions added to cover the guard example string in both the generated workflow wrapper and the help markdown template.
docs/ci-automation.md Opt-out guidance expanded to explicitly mention forks/clones and provide the concrete `vars.REVIEW_APP_PREFIX != ''
lib/github_flow_templates/.github/cpflow-help.md Same clarified opt-out wording applied to the in-repo help markdown, keeping it consistent with the CI automation docs.
CHANGELOG.md New changelog entry added in the correct section describing the fork/clone opt-out clarification.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer / Fork
    participant GH as GitHub Actions
    participant RW as Upstream Reusable Workflow
    participant PR as Pull Request

    Dev->>GH: Commits cpflow-review-app-help.yml wrapper
    GH->>GH: PR opened (pull_request_target: opened)
    alt No if: guard (default — opts in)
        GH->>RW: "calls cpflow-review-app-help.yml@ref"
        RW->>PR: Posts review-app command reference comment
    else if: guard added (fork / unconfigured clone)
        GH->>GH: "Evaluates vars.REVIEW_APP_PREFIX != '' OR vars.CPLN_ORG_STAGING != ''"
        alt Variables set (Control Plane configured)
            GH->>RW: calls upstream workflow
            RW->>PR: Posts comment
        else Variables empty
            GH-->>PR: Job skipped, no comment posted
        end
    end
Loading

Reviews (1): Last reviewed commit: "Clarify review app help opt-out" | Re-trigger Greptile

@claude

claude Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview

A focused, low-risk documentation and template-comment polish pass. No behavioral changes — all changes are to generated file comments, help docs, and their corresponding specs. The PR does exactly what it says.


What's good

  • Consistent across all three surfaces. The opt-out guidance lands identically in docs/ci-automation.md, the generated cpflow-help.md, and the YAML workflow comment, so downstream users see the same wording wherever they look.
  • Concrete example. Replacing the vague "add a wrapper-level if: guard" with the actual expression vars.REVIEW_APP_PREFIX != '' || vars.CPLN_ORG_STAGING != '' makes the guidance actionable without the user having to guess the syntax.
  • Comment indentation is correct. The # if: line (hash + 3 spaces) is carefully formatted so that removing # (4 chars) yields the properly 4-space-indented if: job property — no cleanup needed after uncommenting.
  • Specs cover the new content. Both new expect(…).to include(…) assertions guard against the example guard expression accidentally being dropped from either generated artifact.
  • CHANGELOG entry is appropriately scoped. It matches the terse style of existing entries and is inserted in the right position.

Minor observations (non-blocking)

  1. || vs && semantics. The example guard fires if either variable is set. That's a reasonable default (early-adopters may configure one before the other, and showing help earlier is harmless), and the "adapt" qualifier covers any repo that prefers &&. Worth confirming this is the intended default rather than a copy-paste choice.

  2. Variable type. vars.* resolves to repository/environment variables, which are distinct from secrets. Callers should note that REVIEW_APP_PREFIX and CPLN_ORG_STAGING need to be set as Variables (not Secrets) in their repo settings for the guard to work. The docs don't call this out explicitly, but it's consistent with how the rest of the project uses vars.*.

  3. Spec isolation. The two new assertions are added to existing it blocks that already test several properties of the same files. Given the file is already well-covered, this is fine; just noting that if either block grows much larger, splitting them into focused it blocks per concern would improve failure messages.


Verdict

Looks good to merge. The only reason to revisit before merging is confirming the || default was intentional (observation 1 above).

@justin808 justin808 merged commit 54a6b8a into main May 25, 2026
10 checks passed
@justin808 justin808 deleted the jg-codex/cpflow-release-polish branch May 25, 2026 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant