Finish Release Workflow#1547
Conversation
- Have the pom update workflow trigger when the release has completed. (Instead of being called, since a called workflow can't have greater permissions than the caller.) - Also update the versions in README.md. Signed-off-by: kingthorin <[email protected]>
| # Replace version in README.md look for >, :, ` (greater than, colon, backtick) followed by 2 period number period number and replace it with the leading char and new version | ||
| sed -i -E 's/([\>`\:]{1})2\.[0-9]\.[0-9]/\1'"$VERS"'/g' README.md |
There was a problem hiding this comment.
I'm happy to hear suggestions on a better way to handle this. There are other version identifiers in the README but the three we currently care about fulfill this specific prefix condition.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1547 +/- ##
============================================
- Coverage 92.49% 92.40% -0.10%
+ Complexity 3290 3286 -4
============================================
Files 326 326
Lines 6474 6474
Branches 630 630
============================================
- Hits 5988 5982 -6
- Misses 336 338 +2
- Partials 150 154 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary and Review provided by Claude 3.7 via copilotSummary This pull request addresses issues with the release workflow in the datafaker-net/datafaker repository and fixes issue #1488 titled “Creating a new release doesn’t create a new release.” The PR was submitted by kingthorin (who is also you, as the current user). Changes Made: Changed how the pom update workflow is triggered: Modified .github/workflows/pom_update.yml to trigger automatically when a release workflow completes Changed from using workflow_call (requiring explicit calling) to workflow_run (event-based triggering) This allows the workflow to have its own permissions rather than inheriting from the caller Added README version updating: Added script to extract the current version from pom.xml Added a sed command to update version references in README.md This ensures documentation stays in sync with actual releases Removed redundant workflow call: Removed the explicit call to pom_update workflow from the release-from-tag.yml file This is no longer needed since the workflow now triggers automatically Stats: Added 9 lines Deleted 5 lines Changed 2 files Single commit Review Strengths: Improves automation: The PR enhances the automation of the release process by making the pom update workflow trigger automatically upon release completion. Fixes permissions issue: Addresses a permissions limitation where called workflows can’t have greater permissions than the caller. Enhances documentation synchronization: Ensures README.md version references stay up-to-date with actual releases. Well-tested: The PR author mentions testing these changes in a test repository before submitting. Clean implementation: The changes are minimal and focused on addressing the specific issue. Considerations: The sed command for updating README.md is quite specific, targeting version patterns that start with “>”, “:”, or “`” followed by version numbers. If version numbers appear in other contexts in the README, they may not get updated. The PR includes hard-coded reference to version “2” in the sed pattern. If major version ever changes, this would need updating. Verdict: This PR appears to be a well-tested, focused improvement to the release workflow that addresses the issues raised in #1488. It improves automation, permissions handling, and documentation synchronization with minimal changes to the codebase. The implementation appears solid and properly tested. |
bodiam
left a comment
bodiam
left a comment
There was a problem hiding this comment.
Happy to give this a shot!
3 participants
Fixes #1488
Tested in this junk repo: https://github.com/kingthorin/dftest (which will be removed after this PR is merged).