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(gerrit): not auto-approving if change already had a Code-Review +1 #32818

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

felipecrs
Copy link
Contributor

Changes

The checkIfApproved function was not working as expected when someone already had published a Code-Review +1 vote. It was wrongly assuming that the change was approved in such case, while the change should only be approved if the change had a Code-Review +2 vote.

Note that when o=DETAILED_LABELS is set, the change response from Gerrit API no longer contains the content from o=LABELS, meaning it should only ever be used in this case (when looking for the approvals), otherwise it would break existing code.

Context

I self-host Renovate and I noticed that auto-merge was not working as expected. I realized it was caused by this issue, because for a given project we have here, the same user/robot also votes the change with a Code-Review +1 vote when SonarQube analysis is successful (which is a very known pattern).

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@felipecrs felipecrs marked this pull request as draft November 29, 2024 23:33
@felipecrs felipecrs marked this pull request as ready for review November 29, 2024 23:53
@felipecrs felipecrs marked this pull request as draft November 30, 2024 06:18
@felipecrs felipecrs marked this pull request as ready for review November 30, 2024 07:24
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.

2 participants