Skip to content

Comments

fix(gerrit): defer change creation to createPr()#39250

Open
felipecrs wants to merge 19 commits intorenovatebot:mainfrom
felipecrs:gerrit-fix-create-pr
Open

fix(gerrit): defer change creation to createPr()#39250
felipecrs wants to merge 19 commits intorenovatebot:mainfrom
felipecrs:gerrit-fix-create-pr

Conversation

@felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Nov 12, 2025

TODOs

  • Refactor how labels are handled when updating a change, there's no need to pass them as push options in that case as it's very unlikely to fail and also introduce code hacks
  • Add prTitle to all commitAndPush calls (very important, as otherwise changes could be created with a different title and Renovate no longer find it)
  • Add a warning when updatePr in Gerrit is called with prTitle. That should never happen and this warning may help find spots where commitAndPush is not pushing the commit with the correct prTitle
  • Consider making prTitle mandatory for commitAndPush to also avoid that. Edit: No go: in many cases the prTitle itself is undefined. It would require too much changes.
  • Test everything again

Changes

The thing is: Gerrit has a very different workflow compared to the other platforms. There is simply no concept of branches for code changes, just the changes themselves.

After a lot of thought, I realized this is the best compromise for the Gerrit platform.

This PR defers the creation of the change to the createPr() method, rather than letting the change be created while preparing the branch.

This is better because it lets the standard Renovate flow continue as usual, and closes a few gaps with the Gerrit platform in Renovate.

This PR also refactors part of #35900 and reverts #38288, as these code hacks outside of the Gerrit platform can now be dropped.

Context

Please select one of the below:

AI assistance disclosure

Did you use AI tools to create any part of this pull request?

Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.

  • No — I did not use AI for this contribution.
  • Yes — minimal assistance (e.g., IDE autocomplete, small code completions, grammar fixes).
  • Yes — substantive assistance (AI generated non‑trivial portions of code, tests, or documentation).
  • Yes — other (please describe):

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

The public repository: https://review.gerrithub.io/c/felipecrs/renovate-tutorial/+/1226234

@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch 2 times, most recently from 34af483 to 38b16bd Compare November 14, 2025 02:13
Signed-off-by: Felipe Santos <felipe.santos@ericsson.com>
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch from 0b1cce2 to d7a2601 Compare November 14, 2025 02:52
@felipecrs felipecrs marked this pull request as draft November 14, 2025 02:52
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch 2 times, most recently from 113b707 to 5aa8581 Compare November 14, 2025 03:16
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch from 5aa8581 to 0b491aa Compare November 14, 2025 03:42
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch from e1fdb37 to e512e13 Compare December 15, 2025 14:34
@hanson76
Copy link

Is this being worked on, we have been forced to pin the version of Renovate to 41.167.0 which is the version prior to the change where Renovate stopped adding reviewers.

Looks like this pr has multiple parts, are all necessary to fix that bug or could it be split into several PRs that could be merged individually?

@felipecrs
Copy link
Contributor Author

I'll look into completing this PR tomorrow.

@felipecrs
Copy link
Contributor Author

Sorry, change of plans. I'll look into completing this next week or the other.

@felipecrs felipecrs marked this pull request as ready for review January 30, 2026 21:38
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch from 928e627 to 1ddaf28 Compare January 30, 2026 21:39
@felipecrs felipecrs force-pushed the gerrit-fix-create-pr branch from c6e53b6 to 5acc7f0 Compare January 31, 2026 22:40
@avm99963
Copy link

avm99963 commented Feb 3, 2026

Hey! As requested, I just took a look at the change.

I left some comments that popped up, but both the approach and the entire diff LGTM!

I also thought about an alternative approach while reading your PRs (but I think it is worse than yours):

Renovate's generic action This PR's approach An alternative approach
Create a branch Create a branch locally without pushing Create a WIP change
Create a PR Submit a new change to Gerrit Update the change to be active

It might be helpful to keep this alternative approach in mind in the case that, in the future, Renovate changes in some way such that it requires the branch to exist in the remote after branch creation. Since I don't think this is the case currently, your approach looks much simpler and cleaner!

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 3, 2026

About your alternative approach suggestion:

Renovate has this concept of Draft PRs, which is in my TODO to be implemented in Gerrit through WIP changes.

@avm99963
Copy link

avm99963 commented Feb 3, 2026

Oh, thanks!

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 3, 2026

@avm99963 thanks a lot for your review!

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