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

ci: enable auto-merging of dependabot PRs #151

Merged
merged 2 commits into from
Jan 7, 2025
Merged

ci: enable auto-merging of dependabot PRs #151

merged 2 commits into from
Jan 7, 2025

Conversation

maliroteh-sf
Copy link
Contributor

No description provided.

@maliroteh-sf maliroteh-sf requested a review from a team as a code owner December 19, 2024 19:20
core.setFailed("The 'dependencies' label is missing.");
}

- name: Check if CI passed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding that flow as 'on: pull_request: types: - labeled' would be triggered when pr is added a label. I guess dependabot creates the pr with label in it, so would 'Check if CI passed' always be unsuccessful? might having a timing issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I understood is that auto-merge job will get cancelled if the label dependencies is not on the PR. If the label is there then we move to the next phase which is verifying the PR check statuses. If all of the checks have passed too, only then we attempt to merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there will be timing issue because these are all defined as workflow steps and afaik the steps are executed in series not parallel.


// When dependabot creates PRs, it adds 'dependencies' as a label to the PRs.
// Here we check to see if 'dependencies' label is present
const hasDependenciesLabel = labels.some(label => label.name === 'dependencies');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependencies label gets added to both the roll-up minor/patch PRs and the major version updates. Is there further gating to ensure that this only happens for minor/patch roll-up PRs?
image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From these docs, it looks like we could get more granular based on metadata that comes with the PRs. It also looks like we could do it a bit more streamlined if we leverage the gh CLI as part of the process. Might be worth a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khawkins Oh good point.... I forgot to limit it to minor & patch only. ChatGPT is suggesting to simply look at the title of PR to determine what type of update it is. Since we've already enabled grouping of dependabot PRs for minor & patch, we could simply test to see whether the PR subject includes minor-and-patch group and if so then proceed with auto-merge. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think checking for minor-and-patch group in the subject could be sufficient. It looks like there's a lot of interesting metadata that comes back from the https://github.com/dependabot/fetch-metadata GHA, so check out the README there and see if that changes your mind about your approach. But otherwise, I think checking the subject should be fine.

Copy link
Collaborator

@khawkins khawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maliroteh-sf maliroteh-sf merged commit 3820b5b into main Jan 7, 2025
15 checks passed
@maliroteh-sf maliroteh-sf deleted the auto_merge branch January 7, 2025 23:56
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.

3 participants