ci: cleanup old workflows and test publish on dev branch - #442
Conversation
❌ This PR targets
|
WalkthroughAdds a ChangesCI Workflow Restructuring and Dev-Branch Release Support
GPU Error Message Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
coderabbitai
Bot
added
the
github_actions
❌ This PR targets
|
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 18-19: The permissions section granting `pull-requests: write`
access lacks documentation explaining why this permission is required. Add a
brief explanatory comment above or inline with the `pull-requests: write`
permission line that clarifies the purpose of this write access for future
maintainers. The comment should explain that this permission is needed to allow
the workflow to interact with pull requests (such as updating PR status, adding
comments, or updating checks).
In @.github/workflows/release.yml:
- Around line 36-38: The "Debug release-please outputs" step containing the echo
command for toJSON(steps.rp.outputs) is creating unnecessary noise in production
logs. Either remove this entire step entirely if it's no longer needed, or add a
conditional to the step (using the if directive) to only execute when a DEBUG
environment variable or input parameter is explicitly enabled. This way, the
debug output will only appear when intentionally requested during development.
- Around line 86-90: Add `persist-credentials: false` to all checkout steps in
the workflow to prevent credentials from being persisted to `.git/config`, which
reduces the security risk of credential extraction through artifacts or
subsequent steps. Apply this parameter to the checkout step using
`actions/checkout@900f2210b1d28bbbd0bd22d17926b9e224e8f231` at line 86, the
build-js checkout at line 136, and the Python build job checkouts at lines 189
and 215. Each checkout step should include this new parameter within its `with:`
block to disable credential persistence since none of these jobs require git
push access after checkout.
- Line 5: The YAML formatting contains extra spaces inside the square brackets
in the branches array definition. Remove the spaces after the opening bracket
and before the closing bracket in the branches field so that the array elements
are directly adjacent to the brackets without any whitespace separation.
- Around line 40-47: In the "Set dry-run flag" step with id "dry_run", avoid
directly interpolating the GitHub context variable ${{ github.ref_name }} in the
shell script to prevent template injection. Instead, add an env section to this
step that captures github.ref_name as an environment variable, then reference
that environment variable in the shell conditional logic instead of using the
direct template interpolation syntax.
- Around line 34-47: The dry-run condition in the release-please step (checking
`github.ref_name == 'dev'`) is inconsistent with the condition in the Set
dry-run flag step (checking `github.ref_name != 'main'`). These represent
different logic and would diverge if additional branches are added. Update the
dry-run parameter in the release-please step to use the same condition as the
Set dry-run flag step by changing the condition from `github.ref_name == 'dev'`
to `github.ref_name != 'main'` to ensure both steps have aligned and consistent
dry-run logic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e5cb48e6-1e43-4951-b940-d8c9a915cd22
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/pr-check.yml.github/workflows/release.yml.github/workflows/stale.ymlcore/include/internal/gpu.h
💤 Files with no reviewable changes (2)
- .github/workflows/stale.yml
- .github/workflows/pr-check.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Lint & Validate Code
- GitHub Check: Analyze (c-cpp)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{hpp,cpp,c,h}
📄 CodeRabbit inference engine (.editorconfig)
**/*.{hpp,cpp,c,h}: Use 4-space indentation for C/C++ files
Maintain 120 character maximum line length for C/C++ files
Files:
core/include/internal/gpu.h
**/*.{cpp,cc,cxx,c++,h,hpp,hxx,h++}
📄 CodeRabbit inference engine (.clang-format)
**/*.{cpp,cc,cxx,c++,h,hpp,hxx,h++}: Use LLVM coding style as the base style for C++ code
Use C++20 standard for all C++ code
Use an indent width of 4 spaces for C++ code
Limit line length to 100 columns in C++ code
Use 4 spaces for tab width in C++ code
Use C++11 braced list style with space before braced lists in C++ code
Attach opening braces to the same line (BreakBeforeBraces: Attach) in C++ code
Never pack constructor initializer lists on a single line in C++ code
Break constructor initializer lists before comma in C++ code
Use 4 spaces for constructor initializer list indentation in C++ code
Use block indent alignment after opening brackets in C++ code
Align pointers to the left in C++ code
Regroup and organize include blocks in C++ code
Sort includes case-insensitively in C++ code
Do not indent extern "C" blocks in C++ code
Allow short lambdas only inline on a single line in C++ code
Do not allow short functions on a single line in C++ code
Files:
core/include/internal/gpu.h
core/**/*.{cpp,c,h,hpp}
⚙️ CodeRabbit configuration file
core/**/*.{cpp,c,h,hpp}: This is the Img2Num core C/C++ library. Review for:
- Memory safety: null pointer dereferences, use-after-free, buffer overflows.
- Correct RAII usage and smart pointer idioms.
- Adherence to the .clang-format style; formatting must be applied via
./img2num format-cpp(or./img2num format-wasmfor WASM modules),
NOT by calling clang-format directly.- When suggesting build/test steps, always use the Docker-first wrapper scripts
(./img2num,img2num.ps1, orimg2num.bat) rather than direct tool invocations,
since contributors may not have dependencies installed locally.- Must have Doxygen docstrings in the API (
img2num.h) files. This is important!
Files:
core/include/internal/gpu.h
**/*.{cpp,c,h,hpp}
⚙️ CodeRabbit configuration file
**/*.{cpp,c,h,hpp}: - For any C/C++ code outside core/ (e.g. bindings, example-apps), apply the same
memory-safety and style standards as the core. Formatting via./img2num format-cpp.
- Doxygen docstrings are required.
Files:
core/include/internal/gpu.h
**
⚙️ CodeRabbit configuration file
**: # Contributing to Img2NumWant to contribute to Img2Num? There are a few things you need to know.
We wrote a contribution guide to help you get started.
A few important points:
- Add tests with your PR — new features and bug fixes must include tests where appropriate. PRs without tests are unlikely to be approved.
- Follow the repository's coding style rules.
- Use the issue and PR templates when filing issues or submitting code. Your PR will be rejected if you don't.
If you're unsure what to change, open a discussion and someone will assist you.
Questions?
If you have questions or need help:
- Open a discussion
- Create an issue
- Check existing PRs for ideas
Thank you for improving Img2Num! 🎨🚀
**: BasedOnStyle: LLVM
Standard: c++20--- Basic formatting ---
IndentWidth: 4
ColumnLimit: 100
TabWidth: 4--- Braces ---
Cpp11BracedListStyle: true
SpaceBeforeCpp11BracedList: true
BreakBeforeBraces: Attach--- Braced initializers ---
Cpp11BracedListStyle: true
--- Constructor initializer lists ---
PackConstructorInitializers: Never
BreakConstructorInitializers: BeforeComma
ConstructorInitializerIndentWidth: 4--- Alignment ---
AlignAfterOpenBracket: BlockIndent
--- Pointers ---
PointerAlignment: Left
--- Includes ---
IncludeBlocks: Regroup
SortIncludes: CaseInsensitive--- Extern "C" cleanliness ---
IndentExternBlock: NoIndent
--- Lambdas ---
AllowShortLambdasOnASingleLine: Inline
--- Functions ---
AllowShortFunctionsOnASingleLine: None
**: root = true-------------------------
Global defaults
-------------------------
[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
max_line_lengt...
Files:
core/include/internal/gpu.h
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: GitHub Actions workflows. Review for:
- SHA-pinned action versions for third-party actions (security best practice).
- Secrets accessed only via ${{ secrets.* }} — never hardcoded.
- Least-privilege permissions on each job/workflow.
- Correct job dependency ordering (needs:) and if/condition logic.
Files:
.github/workflows/ci.yml.github/workflows/release.yml
🧠 Learnings (4)
📚 Learning: 2026-02-25T21:24:34.055Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 272
File: core/include/internal/bilateral_filter_gpu.h:108-109
Timestamp: 2026-02-25T21:24:34.055Z
Learning: In WGSL shaders, .rgb swizzle on a vec4 returns the first three components regardless of color space. Do not assume data is RGB color space when the texture contains non-RGB data (e.g., CIELAB L, A, B). If LAB data is stored, treat and compute distances in LAB space, not RGB. Clearly document shader code paths that rely on specific color spaces and prefer explicit conversions or comments when using swizzled components with non-RGB textures. Apply this guidance to WGSL shader files across the codebase (not just this header) when handling color data or color-like channels.
Applied to files:
core/include/internal/gpu.h
📚 Learning: 2026-04-09T19:05:40.514Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 302
File: core/include/internal/gpu.h:203-216
Timestamp: 2026-04-09T19:05:40.514Z
Learning: In Img2Num’s internal GPU initialization code, if querying adapter limits fails (e.g., `adapter.GetLimits(...)` returns `false`), treat this as a GPU initialization failure and route execution to the CPU fallback path. Do not continue with default limits in this case; the failure should cause the same fallback behavior as other GPU init errors.
Applied to files:
core/include/internal/gpu.h
📚 Learning: 2026-05-01T22:50:11.527Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 339
File: release-please-config.json:18-47
Timestamp: 2026-05-01T22:50:11.527Z
Learning: In this repo, release-please-action v4 preserves '/' verbatim in slash-containing path-based package keys when emitting GitHub Actions output names (e.g., `bindings/c--release_created`). When referencing these step outputs in `job.outputs` (and other expressions), use bracket notation with the exact output name: `${{ steps.release.outputs['bindings/c--release_created'] }}` rather than dot notation. If needed, map the complex step output to a clean job-level output alias so downstream jobs can use dot notation via that alias.
Applied to files:
.github/workflows/ci.yml.github/workflows/release.yml
📚 Learning: 2026-05-19T17:30:09.565Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 375
File: .github/workflows/cmake-build.yml:86-88
Timestamp: 2026-05-19T17:30:09.565Z
Learning: In Ryan-Millard/Img2Num CI/workflow YAMLs, any `uv sync` command used for the Python package build must include `--no-build-isolation` (do not remove it). If you need deterministic dependency installs for CI, you may add `--frozen` alongside it (e.g., `uv sync --frozen --no-build-isolation`), and it should not conflict with the repo’s build setup.
Applied to files:
.github/workflows/ci.yml.github/workflows/release.yml
🪛 YAMLlint (1.37.1)
.github/workflows/release.yml
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
🪛 zizmor (1.25.2)
.github/workflows/ci.yml
[warning] 19-19: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[info] 22-22: action functionality is already included by the runner (superfluous-actions): use gh pr comment or gh issue comment in a script step
(superfluous-actions)
.github/workflows/release.yml
[warning] 3-5: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
[info] 38-38: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 43-43: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 120-120: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
[warning] 136-139: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 171-171: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 163-163: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
[info] 157-157: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
[info] 277-277: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[info] 255-255: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
🔇 Additional comments (6)
core/include/internal/gpu.h (1)
232-234: Current code is safe; WebGPU callbacks provide explicit length, not sentinel values.The concern about
WGPU_STRLENsentinel causing unsafe allocation is unfounded for this context. According to WebGPU specifications, whenwgpu::StringViewis passed from WebGPU callbacks (as it is on lines 227–234), it always provides an explicit length and is never a sentinel-terminated string. The conditionmsg.data && msg.length > 0correctly guards against null pointers and zero length, making thestd::string(msg.data, msg.length)constructor safe. This pattern is consistently used elsewhere in the file (lines 189, 200) for the same callback context, and no fix is required.> Likely an incorrect or invalid review comment..github/workflows/ci.yml (1)
14-38: LGTM!.github/workflows/release.yml (4)
119-125: LGTM!
156-174: LGTM!
254-275: LGTM!
277-279: LGTM!Also applies to: 325-327
This separates concerns and allows it to re-run on PR updates like target branch changes.
❌ This PR targets
|
Ryan-Millard
changed the title
dev branch
…hub.com/Ryan-Millard/Img2Num into ci/refactor/cleanup-and-dry-publish-dev
…stifying the workflow
Ryan-Millard
changed the title
dev branchdev branch
|
@coderabbitai why was your review skipped? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get PR details including labels, state, draft status, and review info
gh pr view 442 --repo Ryan-Millard/Img2Num --json title,state,isDraft,labels,author,baseRefName,headRefName,reviewRequests,reviews,comments
echo "---EVENTS---"
# Check for any skip labels or ignore labels
gh api repos/Ryan-Millard/Img2Num/pulls/442/reviews 2>/dev/null | jq '.[] | {user: .user.login, state: .state, body: .body}' 2>/dev/null
echo "---ISSUE COMMENTS---"
gh api repos/Ryan-Millard/Img2Num/issues/442/comments 2>/dev/null | jq '.[] | {user: .user.login, body: .body[:200]}' 2>/dev/nullLength of output: 55944
After that, a new commit ( So the |
|
@coderabbitai please generate a commit message and description for this so I can track it when I merge it. |
|
Tip For best results, initiate chat on the files or code changes.
Commit message: Extended description: If you want to keep it as a single-concern message (CI only) and treat the |
…rect PRs (#442) Refactor and clean up GitHub Actions CI/CD workflows: - Remove `pr-check.yml`: was overly noisy and pedantic; enforced PR title and body format checks that added friction without sufficient value. - Remove `stale.yml`: was already disabled; formally removes the stale issues/PRs automation from the repository. - Update `release.yml`: expand release triggering to both `main` and `dev` branches. On `dev`, all release steps run as dry runs (e.g., npm --dry-run, TestPyPI instead of PyPI). Propagate jobs are gated to non-dry-run runs only. - Create `pr-target-check.yml`: `warn-main-pr` job that posts a warning comment and explicitly fails any pull request that targets `main` directly, enforcing the `dev` → `main` merge flow. It is designed to be annoying. Refactor issue templates: - Remove `good_first_issue.yml`: noisy and unused - Remove `refactor.yml`: noisy and unused - Update `feature_request.yml`: better examples and validation - Update `bug_report.yml`: less noise, more focused, better validation
…rect PRs (#442) Refactor and clean up GitHub Actions CI/CD workflows: - Remove `pr-check.yml`: was overly noisy and pedantic; enforced PR title and body format checks that added friction without sufficient value. - Remove `stale.yml`: was already disabled; formally removes the stale issues/PRs automation from the repository. - Update `release.yml`: expand release triggering to both `main` and `dev` branches. On `dev`, all release steps run as dry runs (e.g., npm --dry-run, TestPyPI instead of PyPI). Propagate jobs are gated to non-dry-run runs only. - Create `pr-target-check.yml`: `warn-main-pr` job that posts a warning comment and explicitly fails any pull request that targets `main` directly, enforcing the `dev` → `main` merge flow. It is designed to be annoying. Refactor issue templates: - Remove `good_first_issue.yml`: noisy and unused - Remove `refactor.yml`: noisy and unused - Update `feature_request.yml`: better examples and validation - Update `bug_report.yml`: less noise, more focused, better validation
1 participant
What was changed & why
CI refactor:
pr-check.yml(noisy, pedantic & no longer wanted)stale.yml(previously disabled)release.yml(includedevbranch, but only do dry runs)ci.yml(fail when the PR targets the main branch)Update
PULL_REQUEST_TEMPLATE.mdto be shorter.Testing & Verification
CI is terrible for this.