harden bump version#2210
Conversation
|
👋 @ofek1weiss |
📝 WalkthroughWalkthroughThe GitHub Actions workflow for version bumping is enhanced with explicit least-privilege permissions, improved version validation using regex-matching instead of sed extraction, consistent environment variables, pinned action commit SHAs for reproducibility, and adjusted job dependencies to use validated version outputs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/bump-version.yml (1)
89-98: Redundant conditional steps can be consolidated.Both "Bump version for package (using input)" and "Bump version for package (using default)" execute identical
sedcommands sinceDBT_PACKAGE_VERSIONis already set with fallback logic at line 74. These two conditional steps can be merged into a single unconditional step.♻️ Proposed consolidation
- name: Bump version for package (using input) - if: ${{ needs.validate-version.outputs.validated-dbt-package-version != ''}} - run: | - sed -i "s/version: [0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*$/version: $DBT_PACKAGE_VERSION/" ./elementary/monitor/dbt_project/packages.yml - sed -i "s/version: [0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*$/version: $DBT_PACKAGE_VERSION/" ./docs/_snippets/quickstart-package-install.mdx - - name: Bump version for package (using default) - if: ${{ needs.validate-version.outputs.validated-dbt-package-version == ''}} + - name: Bump version for package run: | sed -i "s/version: [0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*$/version: $DBT_PACKAGE_VERSION/" ./elementary/monitor/dbt_project/packages.yml sed -i "s/version: [0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*$/version: $DBT_PACKAGE_VERSION/" ./docs/_snippets/quickstart-package-install.mdx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bump-version.yml around lines 89 - 98, Consolidate the two identical steps "Bump version for package (using input)" and "Bump version for package (using default)" into a single step (e.g., "Bump version for package") by removing the conditional checks that reference needs.validate-version.outputs.validated-dbt-package-version and leaving a single run block with the two sed commands that update packages.yml and quickstart-package-install.mdx using the already-populated DBT_PACKAGE_VERSION; this removes duplication while preserving behavior because DBT_PACKAGE_VERSION is set with fallback logic earlier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/bump-version.yml:
- Around line 89-98: Consolidate the two identical steps "Bump version for
package (using input)" and "Bump version for package (using default)" into a
single step (e.g., "Bump version for package") by removing the conditional
checks that reference
needs.validate-version.outputs.validated-dbt-package-version and leaving a
single run block with the two sed commands that update packages.yml and
quickstart-package-install.mdx using the already-populated DBT_PACKAGE_VERSION;
this removes duplication while preserving behavior because DBT_PACKAGE_VERSION
is set with fallback logic earlier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c7ede34-d6d6-49e0-bcb5-2e3ff7d96b1b
📒 Files selected for processing (1)
.github/workflows/bump-version.yml
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
* harden remind-docs-and-tests workflow - SHA-pin wow-actions/auto-comment@v1 - deny GITHUB_TOKEN by default, grant pull-requests:write to the comment job * match elementary-data/elementary#2210 SHA-pin comment convention
Summary by CodeRabbit