-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Shmuelarditi patch 1 #324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shmuelarditi patch 1 #324
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ PR Review
The PR simplifies the workflow by removing dynamic version level determination and hardcoding all releases as patch level. This represents a significant functional change that may impact semantic versioning.
2 issues detected:
🐞 Bug - Dynamic version level determination based on PR labels has been replaced with hardcoded "patch" level
Details: The workflow previously determined release levels (major/minor/patch) based on PR labels, but now hardcodes all releases as patch level. This removes the ability to create major or minor releases through PR labeling, potentially breaking semantic versioning practices and downstream automation that depends on proper version levels.
File:.github/workflows/auto-dispatch.yaml (22-22)🐞 Bug - Removed explicit event type check makes the condition less precise and potentially error-prone
Details: The workflow condition logic has been simplified by removing the explicit check for pull_request event type in the merged PR condition. While this may not cause issues given the current trigger configuration, it makes the condition less precise and could lead to unexpected behavior if workflow triggers are modified in the future.
File:.github/workflows/auto-dispatch.yaml (11-11)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀
| { | ||
| "level": "${{ steps.level.outputs.level }}", | ||
| "level": "patch", | ||
| "source": "${{ github.event_name }}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐞 Bug - Semantic Versioning Lost: Restore the dynamic version level determination logic or implement an alternative mechanism to specify release levels based on PR labels or other criteria.
| "level": "patch", | |
| "level": "${{ github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'major') && 'major' || github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'minor') && 'minor' || 'patch' }}", |
| jobs: | ||
| release-dispatch: | ||
| if: (github.event_name == 'push') || (github.event_name == 'pull_request' && github.event.pull_request.merged == true && github.base_ref == 'master') | ||
| if: github.event_name == 'push' || (github.event.pull_request.merged == true && github.base_ref == 'master') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐞 Bug - Condition Logic Change: Keep the explicit event type check in the condition: github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.merged == true && github.base_ref == 'master')
| if: github.event_name == 'push' || (github.event.pull_request.merged == true && github.base_ref == 'master') | |
| if: github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.merged == true && github.base_ref == 'master') |
✨ PR Description
Purpose: Simplify the auto-dispatch workflow by removing dynamic versioning based on PR labels and hardcoding patch level.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀