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

Fix multiple commits edge case #61

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Fix multiple commits edge case #61

merged 1 commit into from
Apr 29, 2024

Conversation

MuriloDalRi
Copy link
Contributor

@MuriloDalRi MuriloDalRi commented Apr 26, 2024

Adding a commit to a Dependabot PR triggered an error where it expected a commit message in a Dependabot format. It should not proceed if there are multiple commits.

Trello

@MuriloDalRi MuriloDalRi force-pushed the fix-multiple-commits branch 2 times, most recently from 61a620b to 4c12136 Compare April 26, 2024 10:12
Copy link
Contributor

@sengi sengi left a comment

Choose a reason for hiding this comment

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

Do you also need to make it verify that all commits are signed by Dependabot? (or do you already do that? apologies I've only v briefly skimmed the code here)

@MuriloDalRi
Copy link
Contributor Author

Do you also need to make it verify that all commits are signed by Dependabot? (or do you already do that? apologies I've only v briefly skimmed the code here)

That happens here

@sengi
Copy link
Contributor

sengi commented Apr 26, 2024

Do you also need to make it verify that all commits are signed by Dependabot? (or do you already do that? apologies I've only v briefly skimmed the code here)

That happens here

I don't think that does what you think it does 😅

@MuriloDalRi
Copy link
Contributor Author

Do you also need to make it verify that all commits are signed by Dependabot? (or do you already do that? apologies I've only v briefly skimmed the code here)

That happens here

I don't think that does what you think it does 😅

Oh you mean verifying the GPG keys?

lib/policy_manager.rb Outdated Show resolved Hide resolved
@MuriloDalRi MuriloDalRi force-pushed the fix-multiple-commits branch 3 times, most recently from d0bfc25 to 5d1c107 Compare April 26, 2024 11:08
@sengi
Copy link
Contributor

sengi commented Apr 26, 2024

I don't think that does what you think it does 😅

Oh you mean verifying the GPG keys?

Yeah. I don't think it's currently even checking that Dependabot is the only committer. Pretty sure it's only checking that Dependabot raised the PR, which doesn't really tell us much.

@MuriloDalRi
Copy link
Contributor Author

I don't think that does what you think it does 😅

Oh you mean verifying the GPG keys?

Yeah. I don't think it's currently even checking that Dependabot is the only committer. Pretty sure it's only checking that Dependabot raised the PR, which doesn't really tell us much.

That's a good point, I'll raise a new PR to add that check!

@MuriloDalRi
Copy link
Contributor Author

@sengi @ChrisBAshton I've raised #62 to address the commit signing issue. Both PRs are ready for review now.

lib/auto_merger.rb Outdated Show resolved Hide resolved
Swap conditionals around so we check a PR only has one commit first.
@MuriloDalRi MuriloDalRi merged commit 167f427 into main Apr 29, 2024
10 checks passed
@MuriloDalRi MuriloDalRi deleted the fix-multiple-commits branch April 29, 2024 10:35
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