-
Notifications
You must be signed in to change notification settings - Fork 28
post comments with doc build #505
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
Conversation
|
""" WalkthroughThe GitHub Actions workflows were updated by removing the labeler workflow and the documentation build job triggered by the "documentation" label. The documentation build workflow was enhanced to add steps that retrieve the uploaded artifact's download URL and post a comment with this link on the pull request. The labeler configuration for the "documentation" label was also removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions Workflow
participant GitHubAPI as GitHub API
participant PR as Pull Request
Workflow->>GitHubAPI: Upload documentation artifact
Workflow->>GitHubAPI: Query artifacts for current run by name
GitHubAPI-->>Workflow: Return artifact ID
Workflow->>Workflow: Construct artifact download URL
Workflow->>GitHubAPI: Post PR comment with download link
GitHubAPI-->>PR: Display comment with documentation download link
Possibly related PRs
Suggested labels
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #505 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 118 118
Lines 9711 9711
=======================================
Hits 9697 9697
Misses 14 14
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:
|
|
✅ Documentation built: |
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
🧹 Nitpick comments (2)
.github/workflows/test_doc_build.yml (2)
29-30: Retention of only one day is very aggressiveArtefacts disappear after 24 h, making it hard for reviewers (especially in different time-zones) to fetch the docs. Consider a more forgiving window (e.g. 3-7 days) unless space/cost constraints are strict.
54-56: Strip trailing whitespace to satisfy yamllintLine 56 has two trailing spaces, already flagged by the linter.
- Note: You must be logged in to github and a DASDAE member to access the link. + Note: You must be logged in to GitHub and a DASDAE member to access the link.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/labeler.yml(0 hunks).github/workflows/labeler.yml(0 hunks).github/workflows/runtests.yml(0 hunks).github/workflows/test_doc_build.yml(1 hunks)
💤 Files with no reviewable changes (3)
- .github/labeler.yml
- .github/workflows/runtests.yml
- .github/workflows/labeler.yml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/test_doc_build.yml
[error] 56-56: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (ubuntu-latest, 3.10)
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.11)
- GitHub Check: test_build_docs
|
✅ Documentation built: |
|
Hey @ahmadtourei, mind taking a look at this PR? The idea behind this change is to have the CI post a comment with a download link so it is easier to view the built documentation. I also removed the labeling workflow (it kept throwing a weird error) and enabled Code Rabbit to do that instead. I also removed the doc build in the TestCode workflow since we have our own workflow for it now. |
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
🧹 Nitpick comments (2)
.github/workflows/test_doc_build.yml (2)
29-29: Re-evaluate 24-hour artefact retention
retention-days: 1purges the ZIP the very next day, which is a bit tight for reviewers in other time-zones or for follow-up debugging.
Unless storage costs are a pressing concern, consider bumping this to 7–14 days.
46-46: Trim trailing whitespace to satisfy YAML-lintLines 46 and 63 contain stray spaces flagged by YAMLlint. Removing them keeps the workflow lint-clean.
Also applies to: 63-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test_doc_build.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/test_doc_build.yml
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 63-63: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code (macos-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.10)
- GitHub Check: test_build_docs
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code (ubuntu-latest, 3.11)
🔇 Additional comments (1)
.github/workflows/test_doc_build.yml (1)
31-53: Harden the bash snippet withset -euo pipefail(and optionally-x)The script now fails fast when the artefact isn’t found—good!
However, individual commands (curl,jq) can still fail silently and propagate bad data. Enabling strict
mode aborts the step on the first non-zero exit or unbound variable, preventing confusing downstream behaviour.run: | - run_id="${{ github.run_id }}" + set -euo pipefail + + run_id="${{ github.run_id }}" artifact_name="documentation_build_test" repo="${{ github.repository }}"[ suggest_essential_refactor ]
.github/workflows/test_doc_build.yml
Outdated
| - name: Comment with documentation link | ||
| if: steps.artifact_url.outputs.found == 'true' | ||
| uses: peter-evans/create-or-update-comment@v4 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| issue-number: ${{ github.event.pull_request.number }} | ||
| body: | | ||
| ✅ Documentation built: | ||
| 👉 [Download](${{ steps.artifact_url.outputs.url }}) | ||
| Note: You must be logged in to github and a DASDAE member to access the link. |
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.
🛠️ Refactor suggestion
Prevent comment spam by anchoring on an identifier
peter-evans/create-or-update-comment creates a fresh comment on every push unless body-includes (or a comment-id) is supplied. For active PRs this quickly clutters the timeline.
uses: peter-evans/create-or-update-comment@v4
with:
token: ${{ secrets.GITHUB_TOKEN }}
issue-number: ${{ github.event.pull_request.number }}
+ body-includes: "✅ Documentation built:"
body: |
✅ Documentation built:
👉 [Download](${{ steps.artifact_url.outputs.url }})
- Note: You must be logged in to github and a DASDAE member to access the link.
+ Note: You must be logged in to GitHub and a DASDAE member to access the link.This updates the existing comment instead of adding duplicates and fixes the proper casing of “GitHub”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Comment with documentation link | |
| if: steps.artifact_url.outputs.found == 'true' | |
| uses: peter-evans/create-or-update-comment@v4 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| issue-number: ${{ github.event.pull_request.number }} | |
| body: | | |
| ✅ Documentation built: | |
| 👉 [Download](${{ steps.artifact_url.outputs.url }}) | |
| Note: You must be logged in to github and a DASDAE member to access the link. | |
| - name: Comment with documentation link | |
| if: steps.artifact_url.outputs.found == 'true' | |
| uses: peter-evans/create-or-update-comment@v4 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| issue-number: ${{ github.event.pull_request.number }} | |
| body-includes: "✅ Documentation built:" | |
| body: | | |
| ✅ Documentation built: | |
| 👉 [Download](${{ steps.artifact_url.outputs.url }}) | |
| Note: You must be logged in to GitHub and a DASDAE member to access the link. |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 63-63: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/test_doc_build.yml around lines 54 to 63, the current step
creates a new comment on every push, causing comment spam. To fix this, add the
`body-includes` parameter with a unique identifier string from the comment body
to the step's inputs. This will make the action update the existing comment
containing that identifier instead of creating duplicates. Also, correct the
casing of "GitHub" in the comment body to ensure proper branding.
|
✅ Documentation built: |
ahmadtourei
left a 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.
Hey @d-chambers, Looks good! Just need to remove the LabelPRs workflow as well. How did you enable Code Rabbit to do auto-labeling for the PRs that modify docs? Can we have a workflow to test its behavior, or should we test it manually in future PRs?
You can configure it from the code rabbit page: https://app.coderabbit.ai/. Try it out and let me know if you can configure it. I don't think we need to test it. If it adds the wrong label occasionally we can just change it. If it consistently messes up we can look at bringing back the other workflow. |
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
♻️ Duplicate comments (1)
.github/workflows/test_doc_build.yml (1)
70-73: Proper casing of “GitHub” still outstanding
This was flagged previously; “github” should be “GitHub” in the note line.
🧹 Nitpick comments (3)
.github/workflows/test_doc_build.yml (3)
31-45: Leverageupload-artifactoutputs instead of an extra API call
actions/upload-artifact@v4already exposes the uploaded artifact’s ID via theartifact-idoutput. Using it here removes an HTTP round-trip, simplifies the script and avoids a race condition while the artifact list is still being populated.- - name: Generate documentation URL - id: artifact_url - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - run_id="${{ github.run_id }}" - artifact_name="documentation_build_test" - repo="${{ github.repository }}" - - artifact_id=$( - curl -s -H "Authorization: Bearer $GITHUB_TOKEN" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${repo}/actions/runs/${run_id}/artifacts" | - jq --arg NAME "$artifact_name" -r '.artifacts[] | select(.name==$NAME) | .id' - ) + - name: Generate documentation URL + id: artifact_url + run: | + run_id="${{ github.run_id }}" + repo="${{ github.repository }}" + artifact_id="${{ steps.upload_doc.outputs.artifact-id }}" ...You’ll need to give the upload step an
id(e.g.,id: upload_doc) and drop the redundantGITHUB_TOKEN,curl, andjq.
This keeps the workflow leaner and more robust.
46-46: Clean up trailing whitespaceYAML-lint flags this blank line for trailing spaces; trim them to keep the workflow lint-clean.
47-50: Typo: “artefact” → “artifact”Minor spelling nit; stick to GitHub’s “artifact” terminology for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test_doc_build.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/test_doc_build.yml
[error] 46-46: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test_code (macos-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.11)
- GitHub Check: test_code (ubuntu-latest, 3.10)
- GitHub Check: test_build_docs
I deleted the workflow file, it just keeps running because it is in the PR's history. I think it will go away on the next PR. |
|
@d-chambers, Hey Derrick, when you got a moment, can you merge this PR that may potentially resolve the latest release action issue? |
This PR adds to #504 by creating a post in the PR with a link to download the built docs.
Summary by CodeRabbit