Skip to content
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

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Conversation

ankatiyar
Copy link
Contributor

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

  • Read the contributing guidelines
  • Signed off each commit with a Developer Certificate of Origin (DCO)
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

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} " ]]
Copy link
Member

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.

Copy link
Contributor

@SajidAlamQB SajidAlamQB left a 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!

Copy link
Member

@merelcht merelcht left a 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 👍

@ankatiyar ankatiyar enabled auto-merge (squash) February 5, 2024 15:26
@ankatiyar ankatiyar merged commit dcad26e into main Feb 5, 2024
28 checks passed
@ankatiyar ankatiyar deleted the auto-merge-action branch February 5, 2024 15:50
@ankatiyar ankatiyar linked an issue Feb 5, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-merging of sync jobs PR is broken
4 participants