Conversation
BlackYps
added
area: documentation
Contributor
📝 WalkthroughWalkthroughUpdated multiple GitHub Actions workflow references from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Author
|
The URL checker will pass again once this PR is merged |
BlackYps
force-pushed
the
cleanup
branch
2 times, most recently
from
5927fca to
1361ecd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docs-changelog.yaml (1)
59-65:⚠️ Potential issue | 🟠 MajorAdd
repositoryparameter to checkout step for forked pull requests.The
Checkout snippetsstep (line 60–64) defaults to the base repository when therepositoryparameter is omitted. On forked PRs,github.head_refreferences a branch in the fork, not the base repository, causing the checkout to fail.Suggested fix
- name: Checkout snippets uses: actions/checkout@v4 with: + repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} path: fa - ref: ${{ inputs.branch || github.head_ref || github.ref_name}} + ref: ${{ inputs.branch || github.head_ref || github.ref_name }} sparse-checkout: | changelog/snippets🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-changelog.yaml around lines 59 - 65, The "Checkout snippets" step using actions/checkout@v4 fails on forked PRs because it omits the repository parameter; update that checkout step (the step named "Checkout snippets" which calls actions/checkout@v4 and currently sets path, ref and sparse-checkout) to include a repository input set to the PR head repo (e.g. use github.event.pull_request.head.repo.full_name with a fallback to github.repository) so the action checks out the correct forked repository/branch for pull requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-build.yml:
- Around line 53-63: Update the reusable workflow references and documentation
to point to the real file name: change the `uses` values for the jobs
`changelog-develop` and `changelog-beta` from
`.github/workflows/docs-generate-changelog.yaml` to
`.github/workflows/docs-changelog.yaml`, and also update the link in
`docs/development-changelog.md` that references `docs-generate-changelog.yaml`
so it points to `docs-changelog.yaml`; ensure the `uses` entries remain relative
(same directory) and verify the job names `changelog-develop` and
`changelog-beta` still match the intended branches.
In @.github/workflows/docs-changelog.yaml:
- Around line 79-87: The step writes NAME to $GITHUB_ENV but then incorrectly
uses ${{ env.NAME }} in the same run and uses the wrong shell pattern; change
the run to compute REFERENCE and NAME with the shell using the last-path-segment
pattern (${REFERENCE##*/}), call changelog-combine.sh with the shell variable
(quoted as "$NAME"), and expose NAME to subsequent steps via env: NAME if
needed—avoid reading $GITHUB_ENV in the same run and stop using ${{ env.NAME }}
inside the run; reference the REFERENCE and NAME variables and the
changelog-combine.sh invocation when making the change.
---
Outside diff comments:
In @.github/workflows/docs-changelog.yaml:
- Around line 59-65: The "Checkout snippets" step using actions/checkout@v4
fails on forked PRs because it omits the repository parameter; update that
checkout step (the step named "Checkout snippets" which calls
actions/checkout@v4 and currently sets path, ref and sparse-checkout) to include
a repository input set to the PR head repo (e.g. use
github.event.pull_request.head.repo.full_name with a fallback to
github.repository) so the action checks out the correct forked repository/branch
for pull requests.
🪄 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: 7925d1b0-eed0-47df-8603-3667f4ad1085
📒 Files selected for processing (14)
.github/DEVELOPMENT.md.github/workflows/deploy-faf.yaml.github/workflows/deploy-fafbeta.yaml.github/workflows/deploy-fafdevelop.yaml.github/workflows/docs-build.yml.github/workflows/docs-changelog.yaml.github/workflows/scripts/bash/changelog-links.sh.github/workflows/stage-fafbeta.yaml.github/workflows/stage-fafdevelop.yaml.github/workflows/tests.yamlchangelog/snippets/sections/template-footer.mdchangelog/snippets/sections/template-header.mddocs/development-changelog.mddocs/development.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-generate-changelog.yaml:
- Around line 86-89: The inline GitHub expression used to set REFERENCE in the
run block is a script-injection risk; instead pass the branch/ref value via the
job's env and use only shell variable expansion in the script. Update the
workflow to define an environment variable (e.g., REFERENCE) in the step's env
and remove `${{ ... }}` from the run script, then compute NAME from that shell
variable and call changelog-combine.sh $NAME, ensuring only the shell variable
(REFERENCE) is expanded in the run content.
🪄 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: b486a379-3756-4e0b-b674-146bf65d4f5e
📒 Files selected for processing (1)
.github/workflows/docs-generate-changelog.yaml
BlackYps
force-pushed
the
cleanup
branch
2 times, most recently
from
b942406 to
f5166e9
Compare
Contributor
Author
|
I recommend to view the commits individually when reviewing. It makes the changes easier to understand. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the proposed changes
Some repo cleanup:
Testing done on the proposed changes
I tested that the changelog generation workflow still functions
Additional context
This is part of a bigger overhaul of the automation in this repo. More to come
No snippet because there are no changes that are relevant for the players
Checklist