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(platform): compare target refs before updating #23094

Conversation

RahulGautamSingh
Copy link
Collaborator

Changes

  • Before updating the target branch of existing PRs check whether the config.baseBranch is diff from PR target branch

Context

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

@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review July 3, 2023 06:40
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

How does this help?

@rarkins
Copy link
Collaborator

rarkins commented Jul 3, 2023

I'm not seeing a logic change

@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jul 3, 2023

The issue is that Azure throws as error when we try to update targetBranch unnecessarily ie. trying to update it to same value.

New change:
Only add targetBranch to update-options if it is different from current one. This will prevent the error.

@rarkins
Copy link
Collaborator

rarkins commented Jul 3, 2023

I expected this to require a change of logic in workers

@RahulGautamSingh
Copy link
Collaborator Author

I expected this to require a change of logic in workers

I thought the same, and then coded it. After that I realised that the checks in the platform files is needed regardless of change in targetBranch so that we check targetBranch != undefined before passing value to the api call.

So I decidedto make changes to platform files instead of workers and plaftorm files.

@rarkins
Copy link
Collaborator

rarkins commented Jul 3, 2023

Are you saying that targetBranch won't be passed to the platform if it's unchanged? Because that doesn't sound right to me - I thought your code passed it every time

@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jul 3, 2023

Correct, workers change is needed. Feeling stupid now as that is that basic logic change that is must🤦‍♂️

lib/modules/platform/bitbucket-server/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/bitbucket/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
@rarkins rarkins added this pull request to the merge queue Jul 4, 2023
Merged via the queue into renovatebot:main with commit 1700467 Jul 4, 2023
36 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 35.159.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 35.159.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants