-
-
Notifications
You must be signed in to change notification settings - Fork 135
Fix screengrab workflows #1453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix screengrab workflows #1453
Conversation
|
Warning Rate limit exceeded@goruha has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFixed a CLI help-flag spacing in a demo screengrab and updated the Screengrabs CI workflow to set an environment, inject ATMOS_PAGER, add a GitHub App token step, and use that token for PR creation with signed commits and templated messages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Prepare as prepare job
participant Build as build job
participant GHApp as create-github-app-token
participant CreatePR as create-or-update-pull-request
Note over Prepare: workflow starts / determines version
Prepare->>Build: needs.prepare.outputs.version
Note over Build: run make build-all install\n(environment: demo, ATMOS_PAGER="false")
Build->>GHApp: request app token (app-id + private-key)
GHApp-->>Build: outputs.token
Build->>CreatePR: create PR using outputs.token\n(sign-commits, templated commit/message/body, base=main)
CreatePR-->>Build: PR created/updated
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed Checks (1 warning)
✅ Passed Checks (2 passed)
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed Checks (1 inconclusive)
✅ Passed Checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
demo/screengrabs/demo-stacks.txt (1)
1-63: Refine the--helpgrep check to target only demo scripts
Limit the CI check to your shell scripts (e.g.-g 'scripts/demo-stacks/*.sh') so it won’t flag screengrab markdown (like the slug in demo/screengrabs/README.md). Adjust your example to:rg -nP '\S--help\b' -g 'scripts/demo-stacks/*.sh'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
demo/screengrabs/demo-stacks.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/atmos/manifest/1.0.json : Update /pkg/datafetcher/schema/atmos/manifest/1.0.json when adding Atmos configuration options.
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/stacks/stack-config/1.0.json : Update /pkg/datafetcher/schema/stacks/stack-config/1.0.json when adding Atmos configuration options.
Applied to files:
demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/atmos/manifest/1.0.json : Update /pkg/datafetcher/schema/atmos/manifest/1.0.json when adding Atmos configuration options.
Applied to files:
demo/screengrabs/demo-stacks.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (1)
demo/screengrabs/demo-stacks.txt (1)
6-6: LGTM — correct flag forwardingInserting the space ensures
--helpis passed tovalidateas intended. Matches prior guidance that help should work without stack context.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1453 +/- ##
=======================================
Coverage 55.94% 55.95%
=======================================
Files 274 274
Lines 28936 28936
=======================================
+ Hits 16189 16191 +2
+ Misses 10955 10951 -4
- Partials 1792 1794 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/screengrabs.yaml (1)
71-72: Good call disabling paging in CI; consider job-level env + verify accepted values.
- Optional: move
ATMOS_PAGER: "false"to thebuildjob’senv:so all sub-steps/scripts inherit it.- Please confirm Atmos recognizes
ATMOS_PAGER="false"(vs e.g.0,"", orPAGER=cat).Proposed diff:
build: needs: [prepare] runs-on: ubuntu-latest + env: + ATMOS_PAGER: "false" steps: @@ - - name: Run make build-all install - run: | - make -C demo/screengrabs build-all install - git add -A - git status - env: - ATMOS_PAGER: "false" + - name: Run make build-all install + run: | + make -C demo/screengrabs build-all install + git add -A + git status
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/screengrabs.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: build
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
77a23d0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/screengrabs.yaml (2)
38-38: Confirm the 'demo' environment exists and won’t require approvals.Good to scope the job, but if the environment has protection rules (reviewers/wait timer), this workflow may hang on PRs or forks. If it’s just for grouping, consider dropping
environmentor using an unprotected one.
72-74: Nice: disabling the pager; consider setting non-interactive env at the job level.To stabilize outputs across all steps, move
ATMOS_PAGERto the jobenvand add common CI flags.Apply within this step:
- env: - ATMOS_PAGER: "false"Add to the
buildjob (outside this hunk):jobs: build: env: ATMOS_PAGER: "false" CI: "true" TF_INPUT: "0" TF_IN_AUTOMATION: "1"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/screengrabs.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Ensure consistency between CLI help text and website documentation
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (2)
.github/workflows/screengrabs.yaml (2)
75-79: App token step looks good; verify installation and permissions.Inputs
app-id/private-keyare correct and the token output is what you want. Ensure the GitHub App is installed on this repo/org with at leastcontents: writeandpull_requests: write. If runs happen in forks, you may need to setowner:explicitly. Token auto-revocation after the job is expected behavior. (github.com)
84-84: Good switch to App token; commit signing is supported.Using the App token here enables
sign-commits: trueto produce verified bot-signed commits. Repo-level permissions are already set (contents,pull-requestswrite). (github.com)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/screengrabs.yaml (1)
81-95: Gate PR creation on forks to prevent failures.Fork PRs can’t create branches in the base repo; skip this step for forks to keep CI green.
- - name: Create or update PR + - name: Create or update PR + if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }} uses: peter-evans/create-pull-request@v7 with: token: ${{ steps.github-app.outputs.token }} branch: "chore/update-build-screengrabs-for-${{ needs.prepare.outputs.version }}" title: "Update screengrabs for ${{ needs.prepare.outputs.version }}" delete-branch: true sign-commits: true commit-message: | chore: update screengrabs for ${{ needs.prepare.outputs.version }} body: | This PR updates the screengrabs for Atmos version ${{ needs.prepare.outputs.version }}. base: main - labels: "no-release" + labels: "no-release"
♻️ Duplicate comments (1)
.github/workflows/screengrabs.yaml (1)
75-79: Upgrade to create-github-app-token v2; add least-privilege + fork guard.Same ask as prior review; pin to v2, scope permissions, and skip on forks to avoid secretless failures.
- - uses: actions/create-github-app-token@v1 + - uses: actions/create-github-app-token@v2 id: github-app + if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }} with: app-id: ${{ vars.BOT_GITHUB_APP_ID }} private-key: ${{ secrets.BOT_GITHUB_APP_PRIVATE_KEY }} + permission-contents: write + permission-pull-requests: write
🧹 Nitpick comments (2)
.github/workflows/vhs.yaml (2)
17-17: Overlapping runs now allowed; confirm this is intentional.Setting cancel-in-progress: false can queue redundant runs on new commits for the same PR. Consider reverting to true to auto-cancel stale builds.
- cancel-in-progress: false + cancel-in-progress: true
64-64: Matrix runs won’t cancel on newer commits.This can produce duplicate gif commits and summary noise. Prefer cancel-in-progress: true for PR-triggered jobs.
- cancel-in-progress: false + cancel-in-progress: true
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/screengrabs.yaml(3 hunks).github/workflows/vhs.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/screengrabs.yaml
[error] 94-94: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (2)
.github/workflows/screengrabs.yaml (2)
38-38: Verify environment exists.Ensure the “demo” environment is defined (and approvals/secrets configured) or this job will fail.
72-74: Nice: disabling paging for non-interactive run.ATMOS_PAGER=false avoids hung steps in CI. LGTM.
f192aa7
|
These changes were released in v1.190.0-test.1. |
* Fix demo-stacks command * Fix workflow * Use github app for repos * Use github app for repos * Use github app for repos * Use github app for repos * Update screengrabs.yaml * Change cancel-in-progress to false in vhs.yaml * Add 'no-release' label to screengrabs PR * Fix screengrabs workflow * Update website-preview-deploy.yml * Change environment from demo to screengrabs * Change environment from 'screengrabs' to 'demo'
what
screengrabsenvironmentvhsworkflow from canceling runningwhy
vhsworkflows on setno-releaselabel