Revert "Issue #29"#31
Conversation
This reverts commit 25ed4b9.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR removes comprehensive Copilot guidelines and restructures GitHub Actions workflows: relocating the changelog to the hardware directory, adding master_branch environment variables, adjusting trigger conditions with path filters, expanding release automation to include nightly releases with tarball generation, and simplifying version extraction logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Pull request overview
This pull request reverts PR #30 (Issue #29), rolling back changes that were made to GitHub Actions workflows and removing Copilot instructions. The revert affects release automation, documentation building, changelog validation, and code style checking workflows.
Changes:
- Reverts version extraction logic simplifications in release workflow
- Removes "Create and push tag" step from release workflow
- Changes changelog validation to look for file in hardware/ subdirectory
- Adds path filters and nightly release functionality to documentation workflow
- Removes GitHub Copilot instructions file
- Adds environment variable definitions across multiple workflows
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/release.yaml | Reverts simplifications to version extraction logic, removes explicit tag creation step, and removes VERSION variable declarations in shell scripts |
| .github/workflows/documentation.yaml | Adds path filters, changes deployment from GitHub Pages-only to include nightly releases, removes branch restrictions on pull_request trigger |
| .github/workflows/changelog-check.yaml | Changes CHANGELOG.md path from root to hardware/ subdirectory, adds path filters to reduce unnecessary workflow runs |
| .github/workflows/astyle.yaml | Adds master_branch environment variable (set to "master") |
| .github/copilot-instructions.md | Completely removes 502-line file containing project-specific Copilot guidance |
Comments suppressed due to low confidence (1)
.github/workflows/release.yaml:121
- The script references
$VERSIONon line 121, but this shell variable is not defined in this step. Since VERSION is a GitHub Actions environment variable from a previous step, it needs to be accessed as${{ env.VERSION }}or defined at the start of the script asVERSION="${{ env.VERSION }}".
- name: Generate release notes with diff
run: |
# Get the previous release tag (without v prefix)
PREVIOUS_TAG=$(git tag --sort=-v:refname | grep -E '^[0-9]+\.[0-9]+\.[0-9]+' | head -n 1 || true)
echo "RELEASE_NOTES<<EOF" >> $GITHUB_ENV
if [ -n "$PREVIOUS_TAG" ]; then
echo "## Changes" >> $GITHUB_ENV
echo "" >> $GITHUB_ENV
git log $PREVIOUS_TAG..HEAD --pretty=format:"- %s (%h)" >> $GITHUB_ENV || true
echo "" >> $GITHUB_ENV
echo "" >> $GITHUB_ENV
echo "See [CHANGELOG.md](CHANGELOG.md) for full details." >> $GITHUB_ENV
else
echo "## Release $VERSION" >> $GITHUB_ENV
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| paths: | ||
| - 'hardware/CHANGELOG.md' |
There was a problem hiding this comment.
The workflow is configured to validate the changelog at path 'hardware/CHANGELOG.md', but the repository structure shows CHANGELOG.md is located in the root directory, not in a 'hardware/' subdirectory. This will cause the workflow to fail when trying to find the file. The path should be 'CHANGELOG.md' instead of 'hardware/CHANGELOG.md'.
| sed -i -E "s/set\([A-Z_]*VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true | ||
| sed -i -E "s/set\([A-Z_]*LIB_VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true |
There was a problem hiding this comment.
The sed command syntax is incorrect. The pattern s/set\([A-Z_]*VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g will replace the entire matched text with itself (\0) followed by the version, which will result in something like set(VERSION "1.0.0" 1.0.0) instead of replacing the version value. The command should be structured to capture and replace only the version value within the set() call, for example: s/(set\([A-Z_]*VERSION[A-Z_]* \)[^)]*\)/\1\"$VERSION\")/g
| sed -i -E "s/set\([A-Z_]*VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true | |
| sed -i -E "s/set\([A-Z_]*LIB_VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true | |
| sed -i -E "s/(set\([A-Z_]*VERSION[A-Z_]* )[^)]*/\1\"$VERSION\")/g" CMakeLists.txt || true | |
| sed -i -E "s/(set\([A-Z_]*LIB_VERSION[A-Z_]* )[^)]*/\1\"$VERSION\")/g" CMakeLists.txt || true |
| REPO_URL="https://github.com/${{ github.repository }}" | ||
| VERSION="${{ env.VERSION }}" | ||
|
|
||
|
|
There was a problem hiding this comment.
The script references $VERSION on line 62, but this shell variable is not defined in this step. The VERSION was set as a GitHub Actions environment variable in the previous step using echo "VERSION=$VERSION" >> $GITHUB_ENV, which means it needs to be accessed as ${{ env.VERSION }} in the shell script, or the script needs to define a local shell variable at the start like VERSION="${{ env.VERSION }}" to make $VERSION work properly.
| # Load version from GitHub Actions environment into a shell variable | |
| VERSION="${{ env.VERSION }}" | |
| - name: Update CMakeLists.txt | ||
| run: | | ||
| VERSION="${{ env.VERSION }}" | ||
| # Update version in CMakeLists.txt (searches for any VERSION variable) | ||
| sed -i -E "s/set\([A-Z_]*VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true | ||
| sed -i -E "s/set\([A-Z_]*LIB_VERSION[A-Z_]* [^)]*\)/set(\0 $VERSION)/g" CMakeLists.txt || true |
There was a problem hiding this comment.
The script references $VERSION on lines 92-93, but this shell variable is not defined in this step. Since VERSION is a GitHub Actions environment variable from a previous step, it needs to be accessed as ${{ env.VERSION }} or defined at the start of the script as VERSION="${{ env.VERSION }}".
| - name: Update idf_component.yml | ||
| run: | | ||
| VERSION="${{ env.VERSION }}" | ||
| # Update version in idf_component.yml | ||
| sed -i "s/^version: .*/version: \"$VERSION\"/" idf_component.yml |
There was a problem hiding this comment.
The script references $VERSION on line 98, but this shell variable is not defined in this step. Since VERSION is a GitHub Actions environment variable from a previous step, it needs to be accessed as ${{ env.VERSION }} or defined at the start of the script as VERSION="${{ env.VERSION }}".
| paths: | ||
| - 'hardware/CHANGELOG.md' |
There was a problem hiding this comment.
The workflow is configured to watch for changes to 'hardware/CHANGELOG.md', but the CHANGELOG.md file is located in the root directory of the repository, not in a 'hardware/' subdirectory. This path filter will never match changes to the actual CHANGELOG.md file. The path should be 'CHANGELOG.md' instead.
| - name: Validate Changelog Format | ||
| shell: bash | ||
| run: | | ||
| CHANGELOG_FILE="CHANGELOG.md" | ||
|
|
||
| CHANGELOG_FILE="hardware/CHANGELOG.md" | ||
| if [ ! -f "$CHANGELOG_FILE" ]; then | ||
| echo "Error: CHANGELOG.md not found in directory" | ||
| echo "Error: CHANGELOG.md not found in hardware directory" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The script attempts to validate 'hardware/CHANGELOG.md', but this path does not exist in the repository. The CHANGELOG.md file is located in the root directory. This will cause the validation to fail with "Error: CHANGELOG.md not found in hardware directory". The CHANGELOG_FILE variable should be set to "CHANGELOG.md" instead.
| - name: Check for duplicate issue numbers | ||
| shell: bash | ||
| run: | | ||
| CHANGELOG_FILE="CHANGELOG.md" | ||
|
|
||
| CHANGELOG_FILE="hardware/CHANGELOG.md" | ||
There was a problem hiding this comment.
The script attempts to read from 'hardware/CHANGELOG.md', but this path does not exist in the repository. The CHANGELOG.md file is located in the root directory. The CHANGELOG_FILE variable should be set to "CHANGELOG.md" instead of "hardware/CHANGELOG.md".
| env: | ||
| # Master branch (main or master) | ||
| master_branch: master | ||
|
|
There was a problem hiding this comment.
The master_branch environment variable is set to "master" in this workflow, but it's set to "main" in documentation.yaml and changelog-check.yaml. For consistency, all workflows should use the same value. Additionally, this environment variable doesn't appear to be used anywhere in any of the workflows, so it may be unnecessary.
| env: | |
| # Master branch (main or master) | |
| master_branch: master |
Reverts #30
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.