Ci/notify discussions release #787
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/notify-release.yml (2)
31-68: Excellent implementation of GitHub Discussions for release notifications!This new job perfectly aligns with the PR objective of using GitHub Discussions as the main channel for release notifications. Great job on:
- Utilizing the outputs from the
set-envjob- Extracting the changelog for the specific tag
- Creating a well-formatted discussion with relevant links and information
One minor suggestion for improvement:
Consider adding a conditional step to check if the discussion already exists for the given tag. This could prevent duplicate announcements if the workflow is run multiple times for the same release.
Example:
- name: Check if discussion exists run: | existing_discussion=$(gh api graphql -f query=' query($owner:String!, $name:String!, $category:String!, $title:String!) { repository(owner:$owner, name:$name) { discussions(categoryId:$category, first:1, query:$title) { totalCount } } }' -F owner=${{ github.repository_owner }} -F name=${{ needs.set-env.outputs.repo_name }} -F category=${{ env.CATEGORY_ID }} -F title="${{ needs.set-env.outputs.repo_name }} ${{ needs.set-env.outputs.tag }} released!" --jq '.data.repository.discussions.totalCount') if [ "$existing_discussion" -gt 0 ]; then echo "Discussion already exists, skipping creation" exit 0 fi env: GH_TOKEN: ${{ secrets.OPS_TOKEN }}This step would go before the "Create an announcement discussion for release" step.
137-137: Good update to use the centralized tag output!The change in the
update-docs-versionjob is well-implemented:
- Using
${{ needs.set-env.outputs.tag }}for theaxonedVersioninput- This change makes the job compatible with both automatic and manual triggers
- It correctly leverages the output from the
set-envjobThis modification aligns well with the overall workflow improvements and ensures that the correct version is used for documentation updates regardless of how the workflow was triggered.
For consistency with the
update-docsjob, consider renaming theaxonedVersioninput toversion. This would make the input naming consistent across all documentation update jobs.- "axonedVersion": "${{ needs.set-env.outputs.tag }}" + "version": "${{ needs.set-env.outputs.tag }}"However, make sure this change doesn't break any downstream workflows that might be expecting the
axonedVersioninput.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (4 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
20-20: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
79-79: property "set-env" is not defined in object type {}
(expression)
96-96: property "set-env" is not defined in object type {}
(expression)
113-113: property "set-env" is not defined in object type {}
(expression)
133-133: property "set-env" is not defined in object type {}
(expression)
🔇 Additional comments (4)
.github/workflows/notify-release.yml (4)
7-10: Excellent addition of manual workflow trigger!The new
workflow_dispatchtrigger with an optionaltaginput aligns perfectly with the PR objective. This change offers great flexibility by allowing:
- Manual triggering of the workflow
- Specifying a particular release tag or defaulting to the latest one
This enhancement will be particularly useful for replaying the workflow when necessary, as mentioned in the PR objectives.
12-16: Well-structured environment setup job!The renaming of the job to
set-envand the addition oftagandrepo_nameoutputs are excellent changes. They provide a centralized place for setting up crucial environment variables that can be used by subsequent jobs. The logic for handling both manual input and automatic tag detection is sound and aligns well with the new manual trigger functionality.Also applies to: 18-29
83-83: Good update to use the centralized tag output!The changes in the
update-docsjob are well-implemented:
- Consistently using
${{ needs.set-env.outputs.tag }}across all documentation update steps- This change makes the job compatible with both automatic and manual triggers
- It correctly leverages the output from the
set-envjobThese modifications align well with the overall workflow improvements and ensure that the correct version is used for documentation updates regardless of how the workflow was triggered.
Also applies to: 100-100, 117-117
Line range hint
1-139: Excellent overall improvements to the release notification workflow!This updated workflow successfully addresses the PR objectives and introduces several improvements:
- Support for both automatic and manual triggers, enhancing flexibility
- Transition to GitHub Discussions for release notifications, with optional forwarding to Discord
- More modular and reusable structure with centralized environment variable setting
- Consistent use of the release tag across all jobs
These changes will significantly improve the maintainability and usability of the release notification process. Great job on implementing these enhancements!
There are a few minor issues to address:
- Updating the deprecated
set-outputcommands- Fixing the shellcheck quoting issue
- Considering the addition of a check for existing discussions
- Optionally renaming the
axonedVersioninput for consistencyOnce these small adjustments are made, this PR will be in excellent shape for merging.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
20-20: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
79-79: property "set-env" is not defined in object type {}
(expression)
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #787 +/- ##
=======================================
Coverage 42.77% 42.77%
=======================================
Files 108 108
Lines 5078 5078
=======================================
Hits 2172 2172
Misses 2796 2796
Partials 110 110 |
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
ba8c415 to
952e6e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/notify-release.yml (2)
20-32: Enhance script robustness and address shellcheck warningsWhile the script logic is correct, there are some shellcheck warnings that we should address to improve robustness:
- Use double quotes to prevent globbing and word splitting.
- Properly quote expansions inside ${..} to prevent pattern matching.
Here's an improved version of the script:
run: | - if [ -n "$INPUT_TAG" ]; then - TAG="$INPUT_TAG" + if [ -n "${INPUT_TAG}" ]; then + TAG="${INPUT_TAG}" else - TAG=$(gh release view --json tagName -q '.tagName') + TAG="$(gh release view --json tagName -q '.tagName')" fi echo "tag=$TAG" >> $GITHUB_OUTPUT - REPO_NAME=${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/} + REPO_NAME="${GITHUB_REPOSITORY#"${GITHUB_REPOSITORY_OWNER}/"}" echo "repo_name=$REPO_NAME" >> $GITHUB_OUTPUT env: INPUT_TAG: ${{ github.event.inputs.tag }}These changes will address the shellcheck warnings and make the script more robust.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
Line range hint
72-124: LGTM: Consistent update to docs versioningThe changes to the
update-docsjob, particularly the use of${{ needs.set-env.outputs.tag }}for versioning, are consistent with the overall workflow improvements. This ensures that the correct version is used across all documentation updates.Consider refactoring the repeated steps for updating different documentation sections into a reusable workflow or composite action. This would adhere to the DRY (Don't Repeat Yourself) principle and make future maintenance easier. For example:
jobs: update-docs: strategy: matrix: section: [modules, commands, predicates] steps: - name: Update ${{ matrix.section }} docs uses: ./.github/actions/update-docs # Create a composite action with: section: ${{ matrix.section }} version: ${{ needs.set-env.outputs.tag }} repository: ${{ github.repository }} docs_directory: docs/${{ matrix.section == 'modules' && 'proto' || matrix.section }}/*This suggestion is optional and can be considered for future improvements.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
81-81: property "set-env" is not defined in object type {}
(expression)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (4 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
81-81: property "set-env" is not defined in object type {}
(expression)
98-98: property "set-env" is not defined in object type {}
(expression)
115-115: property "set-env" is not defined in object type {}
(expression)
135-135: property "set-env" is not defined in object type {}
(expression)
🔇 Additional comments (4)
.github/workflows/notify-release.yml (4)
7-10: LGTM: Enhanced workflow flexibility with manual triggerThe addition of the
workflow_dispatchtrigger with an optionaltaginput is a great improvement. This change allows for manual invocation of the workflow, providing flexibility to specify a release tag or use the latest one by default. This aligns well with the PR objective of enabling manual triggering of notifications.
12-32: LGTM: Improved job structure and variable handlingThe renaming of the job from 'notify-discord' to 'set-env' and the addition of outputs for 'tag' and 'repo_name' improve the clarity and reusability of this job. The logic for setting the tag and extracting the repo name is sound and aligns with the PR objectives.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
33-70: LGTM: Excellent implementation of GitHub Discussions notificationsThe addition of the
notify-github-discussionjob is a great implementation of the PR objective to update the notification system for releases. This job effectively:
- Uses the outputs from the
set-envjob, ensuring consistency.- Extracts the changelog for the specific release tag.
- Creates a well-formatted discussion post with relevant links and information.
This change will significantly improve the visibility and accessibility of release information for the community.
Line range hint
126-141: LGTM: Consistent update to docs versionThe changes to the
update-docs-versionjob, specifically the use of${{ needs.set-env.outputs.tag }}for theaxonedVersion, are consistent with the overall workflow improvements. This ensures that the correct version is used when updating the docs version in the separate docs repository.🧰 Tools
🪛 actionlint
135-135: property "set-env" is not defined in object type {}
(expression)
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
952e6e7 to
3e89b8c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/notify-release.yml (2)
18-31: LGTM with minor suggestions: Environment variable setupThe script effectively sets up the environment variables, handling both manual input and automatic tag fetching. It aligns well with the workflow_dispatch input and correctly extracts the repository name.
Consider the following minor improvements to address potential shell scripting issues:
Use double quotes around variable expansions to prevent word splitting:
- TAG=$(gh release view --json tagName -q '.tagName') + TAG="$(gh release view --json tagName -q '.tagName')"Quote the variable inside the parameter expansion:
- REPO_NAME=${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/} + REPO_NAME=${GITHUB_REPOSITORY#"${GITHUB_REPOSITORY_OWNER}/"}These changes will make the script more robust against potential issues with special characters or whitespace.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
33-70: LGTM with a minor suggestion: GitHub Discussions notificationThe new
notify-github-discussionjob effectively implements the PR objective of using GitHub Discussions for release notifications. The announcement format is comprehensive and includes all relevant information.One minor suggestion for consistency:
- title: 🎉 ${{ needs.set-env.outputs.repo_name }} ${{ needs.set-env.outputs.tag }} released! + title: 🎉 ${{ github.repository }} ${{ needs.set-env.outputs.tag }} released!This change makes the title consistent with the body of the message, which uses the full repository name. It also ensures that the repository owner is included in the title, which might be beneficial for organizations with multiple repositories.
Overall, great job on implementing this new notification system!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (4 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (6)
.github/workflows/notify-release.yml (6)
7-10: LGTM: Manual trigger with optional tag inputThe addition of the
workflow_dispatchtrigger with an optionaltaginput aligns well with the PR objectives. This change provides the flexibility to manually trigger notifications and specify a particular release tag if needed.
12-12: LGTM: Improved job structure with outputsThe renaming of the job to
set-envand the addition oftagandrepo_nameoutputs improve the workflow's modularity. This change allows other jobs to easily access these values, supporting the updated notification system.Also applies to: 14-16
74-74: LGTM: Conditional execution of docs updateThe addition of the condition
if: github.event_name != 'workflow_dispatch'is a good improvement. It prevents unnecessary documentation updates when the workflow is manually triggered, aligning with the PR objective of allowing manual triggering for specific purposes.
129-129: LGTM: Consistent conditional executionThe addition of
if: github.event_name != 'workflow_dispatch'to theupdate-docs-versionjob is consistent with the similar change in theupdate-docsjob. This prevents unnecessary version updates when the workflow is manually triggered, aligning with the PR objective of allowing manual triggering for specific purposes.
Line range hint
1-143: Overall LGTM: Comprehensive workflow improvementsThis PR successfully implements the objectives outlined, particularly:
- Updating the notification system to use GitHub Discussions instead of Discord.
- Adding the capability for manual triggering of notifications with an optional tag input.
The changes significantly enhance the workflow's functionality and flexibility. The new
notify-github-discussionjob is well-implemented, providing comprehensive release announcements. The addition of theworkflow_dispatchtrigger with the optionaltaginput offers the desired flexibility for manual notifications.While not directly related to the main objectives, the updates to the documentation update process are necessary and well-implemented. They ensure that the documentation remains up-to-date with each release.
Minor suggestions have been made for shell script robustness and consistency in the announcement title. Additionally, verification steps have been suggested for the updated API endpoints to ensure everything is correctly set up in the docs repository.
Great job on these improvements! They will undoubtedly enhance the release notification process and overall workflow efficiency.
🧰 Tools
🪛 actionlint
20-20: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
20-20: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
20-20: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
134-135: LGTM: Updated API endpoint for version updateThe change to the API endpoint for triggering the version update is appropriate and consistent with the changes made in the
update-docsjob.To ensure everything works as expected:
- Verify that the new workflow ID (98246586) is correct for the version update workflow.
- Confirm that the docs repository is set up to handle this new endpoint URL.
#!/bin/bash # Verify the existence of the version update workflow in the docs repository gh api repos/axone-protocol/docs/actions/workflows | jq '.workflows[] | select(.id == 98246586) | .name'
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
3e89b8c to
cffd5db
Compare
We will now use GitHub Discussions as the sole channel for release notifications and forward these to Discord if needed.
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
cffd5db to
503dfb7
Compare
Addresses axone-protocol/community#6.
I manually ran the job on v10.0.0 to ensure everything works properly. You can check the result here: axone-protocol/discussions/7.
Summary by CodeRabbit
New Features
Improvements