-
Notifications
You must be signed in to change notification settings - Fork 906
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
Update auto-merge action to use bash script #3595
Conversation
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
echo "PR ${pr} mergeable: ${mergeable}" | ||
mergeable_state=$(echo "${response}" | tr '\r\n' ' ' | jq --raw-output ".mergeable_state // \"unknown\"") | ||
echo "PR ${pr} mergeable_state: ${mergeable_state}" | ||
[ "${mergeable}" == true ] && [[ " ${VALID_MERGEABLE_STATES[@]} " =~ " ${mergeable_state} " ]] |
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.
Shellcheck complains about this line:
$ shellcheck myscript
[Line 44:](javascript:setPosition(44, 36))
[ "${mergeable}" == true ] && [[ " ${VALID_MERGEABLE_STATES[@]} " =~ " ${mergeable_state} " ]]
^-- [SC2199](https://www.shellcheck.net/wiki/SC2199) (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
>> ^-- [SC2076](https://www.shellcheck.net/wiki/SC2076) (warning): Remove quotes from right-hand side of =~ to match as a regex rather than literally.
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.
Happy to go back to using the old script for this, thank you @ankatiyar!
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.
Thanks for investigating this! It's a shame the action didn't work, but our old script is a good solution 👍
Description
Related #3588
The auto-merge action from the marketplace was unable to get around the "Squash merge" requirement which can only be set on repo level but not branch level.
Development notes
This PR is to update the action to go back to the bash script we were using before with CircleCI - https://github.com/kedro-org/kedro/blob/0.18.14/tools/circleci/github_scripts/attempt_merge_pr.sh
I've been able to make this work on my fork -
We also don't need the "labeler" action anymore since the script only merges PRs on
merge-main-to-develop
branch.Will need to keep an eye on the action for permission issues once it is merged - https://www.designcise.com/web/tutorial/how-to-fix-github-action-permission-denied-error-when-executing-a-file
Checklist
RELEASE.md
file