Automatic changelog and version creation by triggering a new github release #143
Conversation
WalkthroughThe pull request introduces several modifications to GitHub Actions workflow files. The Changes
Possibly related PRs
Suggested reviewers
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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (5)
- .github/workflows/builder.yaml (0 hunks)
- .github/workflows/lint.yaml (0 hunks)
- .github/workflows/release-notes.yaml (1 hunks)
- .github/workflows/tester.yaml (1 hunks)
- ebusd/CHANGELOG.md (1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/builder.yaml
- .github/workflows/lint.yaml
🧰 Additional context used
🪛 actionlint
.github/workflows/release-notes.yaml
22-22: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
22-22: shellcheck reported issue in this script: SC2001:style:8:11: See if you can use ${variable//search/replace} instead
(shellcheck)
22-22: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:10:54: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:11:30: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:12:22: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:13:18: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:14:15: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
48-48: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
.github/workflows/release-notes.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 16-16: trailing spaces
(trailing-spaces)
🪛 Markdownlint
ebusd/CHANGELOG.md
4-4: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (7)
.github/workflows/tester.yaml (1)
6-6: Verify alignment with testing strategy and PR objectivesThe change to trigger the workflow only on the
mainbranch aligns with the PR objective of changing how releases are triggered. However, this modification raises some concerns:
- Reduced testing frequency: By only running on the
mainbranch, we might miss catching issues earlier in the development process on feature branches.- Workflow name inconsistency: The workflow is named "Build and Test Add-on", but it now only runs on
main, which might not align with a comprehensive testing strategy.Consider the following:
- Is this change intentional, or should we maintain testing on feature branches as well?
- Should we rename the workflow to reflect its new purpose (e.g., "Build and Test Add-on for Release")?
- Do we need to implement additional safeguards or testing workflows for feature branches?
To verify the impact of this change, you can run the following script:
This script will help identify if there are other workflows that might be handling testing for feature branches, ensuring we maintain a robust testing strategy across the development process.
.github/workflows/release-notes.yaml (5)
13-15: LGTM! Job setup is well-defined.The job name "release-notes" is descriptive and appropriate. Using
ubuntu-latestensures the workflow runs on an up-to-date environment.
1-76: Overall, the workflow is well-structured but has room for improvement.The "Create Changelog and Release" workflow effectively automates the process of updating changelogs and triggering releases. Here's a summary of the main points for improvement:
- Enhance error handling in shell scripts, particularly in the release info extraction and file modification steps.
- Review and potentially limit the permissions granted to the workflow.
- Consider the implications of pushing changes directly to the main branch and evaluate if a pull request-based approach might be more appropriate.
- Verify the existence and correct configuration of the
builder.yamlworkflow that this workflow attempts to trigger.- Improve the robustness of
sedcommands when handling release notes with special characters.Addressing these points will significantly enhance the reliability and security of the workflow. Great job on automating this process, and I look forward to seeing the improvements!
🧰 Tools
🪛 actionlint
22-22: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
22-22: shellcheck reported issue in this script: SC2001:style:8:11: See if you can use ${variable//search/replace} instead
(shellcheck)
22-22: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:10:54: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:11:30: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:12:22: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:13:18: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:14:15: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
48-48: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 16-16: trailing spaces
(trailing-spaces)
9-11: LGTM! Consider reviewing the necessity of both permissions.The permissions granted are appropriate for the workflow's tasks.
However, it's a good practice to limit permissions to the minimum required. Please verify if both
contents: writeandactions: writeare necessary for this workflow. If you can confirm that both are required, you can disregard this comment.#!/bin/bash # Description: Check if both permissions are used in the workflow # Test 1: Check for operations that require contents:write echo "Checking for contents:write usage:" grep -n -E "git (add|commit|push)" .github/workflows/release-notes.yaml # Test 2: Check for operations that require actions:write echo "Checking for actions:write usage:" grep -n -E "(workflow_dispatch|repository_dispatch)" .github/workflows/release-notes.yaml
70-76: LGTM. Verify the existence and configuration of the builder workflow.The approach to trigger another workflow using the GitHub API is valid and well-implemented.
To ensure the reliability of this step, please verify:
- The existence of the
builder.yamlworkflow file.- The correct configuration of the
builder.yamlworkflow to acceptworkflow_dispatchevents.You can use the following script to check for the existence of the
builder.yamlfile and its configuration:This verification will help ensure that the triggered workflow exists and is properly configured to be dispatched by this action.
✅ Verification successful
Verified: Builder workflow exists and is properly configured for
workflow_dispatchevents.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and configuration of the builder workflow # Check if builder.yaml exists if [ -f ".github/workflows/builder.yaml" ]; then echo "builder.yaml exists." # Check if builder.yaml is configured for workflow_dispatch if grep -q "workflow_dispatch:" ".github/workflows/builder.yaml"; then echo "builder.yaml is configured for workflow_dispatch events." else echo "Warning: builder.yaml does not seem to be configured for workflow_dispatch events." fi else echo "Warning: builder.yaml does not exist in the expected location." fiLength of output: 277
38-54:⚠️ Potential issueImprove error handling and consider potential sed issues.
The use of
sedfor file modifications is appropriate, and including error checking is good practice. However, there are some improvements we can make:
- Replace
$?checks with direct command result checks.- Consider potential issues with special characters in release notes affecting
sedoperation.Here's an improved version for both steps:
# Append release notes to changelog.md if ! sed -i "/<!-- append new version here -->/ a ## ${{ env.VERSION_WITH_PREFIX }}\n${{ env.NOTES }}\n" \ "${{ github.workspace }}/ebusd/CHANGELOG.md"; then echo "Failed to update CHANGELOG.md" exit 1 fi # Update version in config.yaml if ! sed -i "s/version: .*/version: \"${{ env.VERSION }}\"/" \ "${{ github.workspace }}/ebusd/config.yaml"; then echo "Failed to update config.yaml" exit 1 fiAdditionally, consider testing the
sedcommands with various types of release notes to ensure they handle special characters correctly. You might want to escape special characters in${{ env.NOTES }}before using it in thesedcommand.This script will help verify if the
sedcommand can handle various types of release notes correctly.✅ Verification successful
Release note handling verified successfully.
All tested
sedcommands handle special characters and multi-line content correctly. Additionally, adopting direct command result checks can further enhance script robustness.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Test sed command with various types of release notes # Test cases test_notes=( "Simple release note" "Release note with special characters: $, /, &" "Multi-line release note with newlines and * wildcards" ) for note in "${test_notes[@]}"; do echo "Testing with note: $note" if ! echo "<!-- append new version here -->" | sed "/<!-- append new version here -->/ a ## v1.0.0\n$note\n"; then echo "Failed for note: $note" fi echo "---" doneLength of output: 651
Script:
#!/bin/bash # Description: Corrected script to test sed command with various types of release notes # Initialize a temporary CHANGELOG.md file echo "<!-- append new version here -->" > temp_CHANGELOG.md # Test cases test_notes=( "Simple release note" "Release note with special characters: \$, /, &" $'Multi-line release note\nwith newlines and * wildcards' ) for note in "${test_notes[@]}"; do echo "Testing with note: $note" if sed "/<!-- append new version here -->/ a ## v1.0.0\n$note\n" temp_CHANGELOG.md > updated_CHANGELOG.md; then echo "Success for note: $note" else echo "Failed for note: $note" fi echo "---" doneLength of output: 1431
🧰 Tools
🪛 actionlint
39-39: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
48-48: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
ebusd/CHANGELOG.md (1)
1-4: Improved changelog structure and formattingThe overall structure and formatting of the changelog have been improved:
- The reference to Home Assistant developer documentation is helpful.
- The placeholder for appending new versions (line 3) is a good practice for maintaining the changelog.
- The version headers have been reformatted for better readability and consistency.
These changes align with best practices for changelog maintenance.
🧰 Tools
🪛 Markdownlint
4-4: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- .github/workflows/release-notes.yaml (1 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/release-notes.yaml (3)
Learnt from: cociweb PR: LukasGrebe/ha-addons#143 File: .github/workflows/release-notes.yaml:18-36 Timestamp: 2024-10-18T01:59:29.320Z Learning: The user prefers to keep the current code in the "Extract release notes and version" step in the `.github/workflows/release-notes.yaml` file as is, since it was previously suggested by me.Learnt from: cociweb PR: LukasGrebe/ha-addons#143 File: .github/workflows/release-notes.yaml:56-68 Timestamp: 2024-10-18T01:56:49.838Z Learning: In this project, pushing changes directly to the `main` branch from GitHub Actions workflows is intentional and aligns with the team's workflow.Learnt from: cociweb PR: LukasGrebe/ha-addons#142 File: .github/workflows/release-notes.yaml:54-67 Timestamp: 2024-10-18T00:58:16.306Z Learning: In the 'release-notes.yaml' workflow, direct pushes to the 'main' branch are intentional for this project.
🪛 actionlint
.github/workflows/release-notes.yaml
22-22: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
22-22: shellcheck reported issue in this script: SC2001:style:8:11: See if you can use ${variable//search/replace} instead
(shellcheck)
22-22: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:10:54: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:11:30: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:12:22: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:13:18: Double quote to prevent globbing and word splitting
(shellcheck)
22-22: shellcheck reported issue in this script: SC2086:info:14:15: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
48-48: shellcheck reported issue in this script: SC2181:style:3:6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
.github/workflows/release-notes.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 16-16: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/release-notes.yaml (1)
1-12: 🧹 Nitpick (assertive)LGTM! Fix minor formatting issue.
The workflow configuration is well-structured with appropriate permissions and trigger conditions. However, there's a minor formatting issue to address.
Fix the trailing whitespace:
jobs: release-notes: runs-on: ubuntu-latest - + steps:Likely invalid or redundant comment.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
|
@tjorim, could you please apply this patch to your updated fork and then create a new github release (with tag pattern of v0.0.0/V0.0.0/0.0.0) ? To ensure the PR does not introduce further issues according to the patch, consider making a release prior to applying the patch as well. Unfortunately, Lukas unable to test it, as it impacts the main branch. I've tested it previously on my main branch, but a confirmation would be beneficial. As part of the pass criteria, the CHANGELOG.md must be updated to include the body text of the new release in markdown format, and the config.yaml must reflect the tag name in its version record. |
|
the ebusd 24.1 is in the tube. Can we merge this before we release that? |
|
I should have some time tonight or tomorrow to test out this PR so we are ready to ship once the alpine package bump is merged. |
|
@tjorim Do You have any feedback? |
There was a problem hiding this comment.
I could see the use case but not convinced of the benefit.
Just tried it once use the 'Generate release notes' when drafting a new release, and it already failed because I used a * for formatting? https://github.com/tjorim/ebusd-ha_addons-cociweb/actions/runs/12165947177/job/33931075696
For me the extra code that could break don't tip the scale over just copying and pasting it into a file after drafting a release.
| branches: | ||
| - main |
There was a problem hiding this comment.
| branches: | |
| - main |
I guess we also don't need this.
| @@ -0,0 +1,76 @@ | |||
| name: Create Changelog and Release | |||
| # Trigger on tags matching the format: v0.0.0, V0.0.0, or 0.0.0 | |||
There was a problem hiding this comment.
Do we want to trigger on all of these?
I think for consistency we should stick to a single format.
Or maybe just switch to Calendar Versioning like HA itself did?
There was a problem hiding this comment.
with #159 i updated the versioning to follow ebusd's ..
|
@tjorim Thanks for checking this. I'm afraid, that the relase 'This is a test release' does not fit to the pattern of x.x.x (which is required to parse the version into the config file and to the release notes.) |
This comment was marked as resolved.
This comment was marked as resolved.
This PR enables appending the changelog.md with the content (body) of a new GitHub Release. Consequently, building and publishing can be initiated by creating a new release (tagged with v0.0.0/V0.0.0/0.0.0 format) or manually through the dispatcher. Publishing will no longer be triggered by merges or pushes.
ATTENTION!
Unfortunatelly, I'm not able to make a final test on my main branch, because the coderabbit 'hops' all the commit into this PR.
Addresses issue #163
Summary by CodeRabbit