Ci/notify discussions release #643
Conversation
WalkthroughThe workflow file 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/notify-release.yml (2)
13-32: LGTM: Environment setup job added with minor improvements neededThe new
set-envjob effectively centralizes environment variable setting and improves modularity. It handles both manual input and automatic tag retrieval, which aligns with the workflow's flexibility requirements.However, there are some minor improvements that can be made to the shell script:
Consider applying the following changes to address shellcheck warnings:
if [ -n "$INPUT_TAG" ]; then - TAG="$INPUT_TAG" + 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 +echo "tag=${TAG}" >> "${GITHUB_OUTPUT}" -REPO_NAME=${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/} -echo "repo_name=$REPO_NAME" >> $GITHUB_OUTPUT +REPO_NAME="${GITHUB_REPOSITORY#"${GITHUB_REPOSITORY_OWNER}/"}" +echo "repo_name=${REPO_NAME}" >> "${GITHUB_OUTPUT}"These changes will prevent potential issues with word splitting and globbing, and ensure proper quoting of variables.
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
36-77: LGTM: GitHub Discussions notification job updatedThe
notify-github-discussionjob has been effectively updated to work with the newset-envjob and improve the notification content. The changes align well with the PR objective of shifting to GitHub Discussions for release notifications.For consistency, consider updating line 45 to use double quotes for the GitHub CLI command:
-gh release view ${{ needs.set-env.outputs.tag }} --json body -q '.body' +gh release view "${{ needs.set-env.outputs.tag }}" --json body -q '.body'This change ensures that the tag is properly quoted, which is especially important if it contains spaces or special characters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (2 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (4)
.github/workflows/notify-release.yml (4)
7-11: LGTM: Manual workflow trigger addedThe addition of the
workflow_dispatchtrigger with an optionaltaginput aligns well with the PR objective of allowing manual triggering of notifications for releases. This provides flexibility for replaying the workflow when necessary, alongside the existing release trigger.
81-81: LGTM: Docs update skipped on manual dispatchThe addition of the condition
if: github.event_name != 'workflow_dispatch'is a good improvement. It prevents unnecessary documentation updates when manually triggering the workflow, which aligns with the PR objective of allowing manual notifications without side effects.
103-103: LGTM: Schema update skipped on manual dispatchThe addition of the condition
if: github.event_name != 'workflow_dispatch'is a good improvement. It prevents unnecessary schema updates when manually triggering the workflow, which aligns with the PR objective of allowing manual notifications without side effects.
Line range hint
1-120: Overall: Excellent improvements to the release notification workflowThe changes in this file successfully implement the PR objectives:
- Removal of Discord notifications: The workflow now focuses on GitHub Discussions for release notifications.
- Manual triggering capability: The new
workflow_dispatchtrigger allows for manual invocation with an optionaltaginput.- Streamlined communication: The
notify-github-discussionjob has been updated to provide comprehensive release information in GitHub Discussions.The restructuring of the workflow improves modularity and clarity, particularly in how environment variables are handled and how notifications are formatted. The addition of conditions to skip documentation and schema updates on manual dispatch prevents unnecessary side effects.
These changes effectively shift the focus to GitHub Discussions for release notifications while maintaining flexibility for manual triggers. The workflow now provides a more streamlined and efficient process for communicating releases within the community.
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
We will now use GitHub Discussions as the sole channel for release notifications and forward these to Discord if needed.
size-limit report 📦
|
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
1968c65 to
32bf8e6
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)
13-32: Great job centralizing environment setup!The new
set-envjob effectively centralizes environment variable setup and output exposure. It handles both manual input and automatic tag retrieval, which is very useful.However, there are some minor improvements we can make to the shell script:
Consider applying these changes to address shellcheck warnings and improve script robustness:
if [ -n "$INPUT_TAG" ]; then - TAG="$INPUT_TAG" + 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 +echo "tag=${TAG}" >> "${GITHUB_OUTPUT}" -REPO_NAME=${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/} -echo "repo_name=$REPO_NAME" >> $GITHUB_OUTPUT +REPO_NAME="${GITHUB_REPOSITORY#"${GITHUB_REPOSITORY_OWNER}/"}" +echo "repo_name=${REPO_NAME}" >> "${GITHUB_OUTPUT}"These changes ensure proper quoting and prevent potential issues with word splitting or globbing.
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
36-77: Excellent improvements to the GitHub Discussions notification!The updates to the
notify-github-discussionjob are well-structured and improve the overall functionality:
- Proper dependency on the
set-envjob ensures correct sequencing.- Addition of the repository checkout step provides flexibility for future enhancements.
- Changelog extraction now correctly uses the tag from
set-env.- The discussion creation step has been significantly improved with a more informative title and body.
For consistency, consider updating the
bodysection to useneeds.set-env.outputs.taginstead ofgithub.event.release.tag_name:-📄 Changelog: <https://github.com/${{ github.repository }}/releases/tag/${{ needs.set-env.outputs.tag }}> +📄 Changelog: <https://github.com/${{ github.repository }}/releases/tag/${{ needs.set-env.outputs.tag }}>This change ensures that the correct tag is always used, whether the workflow was triggered manually or automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (2 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (4)
.github/workflows/notify-release.yml (4)
7-11: Excellent addition of manual workflow trigger!The new
workflow_dispatchtrigger enhances the flexibility of this workflow by allowing manual invocation. The optionaltaginput is particularly useful for specifying a particular release tag when needed.
81-81: Good addition of execution condition!The condition
if: github.event_name != 'workflow_dispatch'is a smart addition. It prevents unnecessary documentation updates when the workflow is manually triggered, avoiding potential unintended side effects.
103-103: Consistent application of execution condition!The addition of
if: github.event_name != 'workflow_dispatch'to theupdate-schemajob is consistent with the approach taken in theupdate-docsjob. This ensures that schema updates are only triggered by actual releases, not manual workflow runs.
Line range hint
1-118: Overall excellent improvements to the notify-release workflow!This update significantly enhances the workflow's flexibility, maintainability, and functionality:
- Addition of manual triggering with optional tag input.
- Centralized environment setup with the new
set-envjob.- Improved GitHub Discussions notification with better formatting and content.
- Consistent handling of manual triggers for docs and schema updates.
These changes collectively make the workflow more robust and user-friendly. The code is well-structured, and the use of environment variables and secrets is appropriate. The minor suggestions provided in previous comments will further improve the script's robustness and consistency.
Great job on this refactoring!
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
ccamel
force-pushed
the
ci/notify-discussions-release
branch
from
32bf8e6 to
38f4a0a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/notify-release.yml (1)
13-32: LGTM! Consider improving shell script robustness.The
set-envjob effectively centralizes the logic for setting important environment variables and handles both manual input and automatic tag retrieval. This structure improves the workflow's modularity and clarity.To enhance the shell script's robustness, consider the following improvements:
- Quote the
$INPUT_TAGvariable to prevent word splitting:-if [ -n "$INPUT_TAG" ]; then - TAG="$INPUT_TAG" +if [ -n "${INPUT_TAG}" ]; then + TAG="${INPUT_TAG}"
- Quote the
${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/}expansion:-REPO_NAME=${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/} +REPO_NAME="${GITHUB_REPOSITORY#${GITHUB_REPOSITORY_OWNER}/}"These changes will make the script more resilient to potential issues with special characters or spaces in the variables.
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/notify-release.yml (2 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/notify-release.yml
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (5)
.github/workflows/notify-release.yml (5)
7-11: LGTM! Great addition for manual workflow triggering.The new
workflow_dispatchtrigger with an optionaltaginput aligns perfectly with the PR objective of allowing manual triggering of notifications for releases. This addition provides flexibility and control over the notification process.
36-77: LGTM! Excellent improvements to the notification process.The updates to the
notify-github-discussionjob significantly enhance the notification process:
- The dependency on
set-envensures access to the required outputs, improving modularity.- The addition of the repository checkout step allows for proper access to release information.
- The changelog extraction now correctly uses the
tagoutput fromset-env.- The announcement discussion format has been improved with more detailed and structured information.
- The use of environment variables for sensitive information (e.g.,
DISCORD_URL,TWITTER_URL) is a good security practice.These changes align well with the PR objectives and improve the overall quality of the release notification process.
81-81: LGTM! Good addition to prevent unnecessary doc updates.The condition
if: github.event_name != 'workflow_dispatch'ensures that theupdate-docsjob only runs for automatic releases and not during manual notification triggers. This change aligns with the PR objectives and prevents unnecessary documentation updates when manually replaying the workflow.
103-103: LGTM! Good addition to prevent unnecessary schema updates.The condition
if: github.event_name != 'workflow_dispatch'ensures that theupdate-schemajob only runs for automatic releases and not during manual notification triggers. This change aligns with the PR objectives and prevents unnecessary schema updates when manually replaying the workflow.
Line range hint
1-118: Overall, excellent improvements to the release notification workflow!The changes in this file successfully achieve the PR objectives:
- Manual triggering of notifications is now possible with the
workflow_dispatchevent.- The notification process has been improved and centralized with the new
set-envjob.- The GitHub Discussions announcement has been enhanced with more detailed information.
- Unnecessary updates to docs and schema during manual triggers have been prevented.
These changes improve the flexibility, clarity, and efficiency of the release notification process. The only minor suggestion is to improve the shell script robustness in the
set-envjob, as mentioned in a previous comment.Great work on this update!
🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2086:info:6:20: Double quote to prevent globbing and word splitting
(shellcheck)
21-21: shellcheck reported issue in this script: SC2295:info:8:31: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns
(shellcheck)
21-21: shellcheck reported issue in this script: SC2086:info:9:32: Double quote to prevent globbing and word splitting
(shellcheck)
Addresses axone-protocol/community#6.
Summary by CodeRabbit
New Features
Improvements