GODRIVER-3797 Skip manual merge on dry run.#101
GODRIVER-3797 Skip manual merge on dry run.#101qingyang-hu merged 2 commits intomongodb-labs:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a dry run check to skip the manual merge operation in the Go driver's pre-publish action. The change prevents the action from performing git merge and push operations when running in dry run mode, which is consistent with the behavior of similar actions in the codebase.
Changes:
- Added
inputs.dry_run == 'false'condition to the "Manually merge up changes" step to prevent merge operations during dry runs
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ignoredBranches: ${{ inputs.ignored_branches }} | ||
| - name: "Manually merge up changes" | ||
| if: ${{ inputs.push_changes == 'true' && ! contains(fromJSON(inputs.ignored_branches), github.ref_name) && steps.get-next-branch.outputs.hasNextBranch }} | ||
| if: ${{ inputs.dry_run == 'false' && inputs.push_changes == 'true' && ! contains(fromJSON(inputs.ignored_branches), github.ref_name) && steps.get-next-branch.outputs.hasNextBranch }} |
There was a problem hiding this comment.
The dry_run input is used in the conditional but is not defined in the inputs section of this action. This will cause the action to fail when invoked.
To fix this, add the dry_run input definition to the inputs section, similar to how it's defined in other actions in the codebase (e.g., python/pre-publish/action.yml:18-20, ruby/publish/action.yml:23-25). The input should be required and have a description explaining its purpose.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ignoredBranches: ${{ inputs.ignored_branches }} | ||
| - name: "Manually merge up changes" | ||
| if: ${{ inputs.push_changes == 'true' && ! contains(fromJSON(inputs.ignored_branches), github.ref_name) && steps.get-next-branch.outputs.hasNextBranch }} | ||
| if: ${{ inputs.dry_run == 'false' && inputs.push_changes == 'true' && ! contains(fromJSON(inputs.ignored_branches), github.ref_name) && steps.get-next-branch.outputs.hasNextBranch }} |
There was a problem hiding this comment.
Consider also adding the dry_run check to the "Determine branch to merge up to" step condition (line 32) to avoid unnecessary computation. When dry_run is true, this step will execute but its output won't be used since the merge step is now skipped. The condition on line 32 could become: inputs.dry_run == 'false' && inputs.push_changes == 'true' && ! contains(fromJSON(inputs.ignored_branches), github.ref_name) for consistency and efficiency.
3 participants
GODRIVER-3797
Skip manual merge on dry run.