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

feat: use cached configHash to prevent branch updates #16661

Merged
merged 91 commits into from Aug 22, 2022

Conversation

RahulGautamSingh
Copy link
Collaborator

@RahulGautamSingh RahulGautamSingh commented Jul 20, 2022

Changes

  • Use a combination of baseBranchSha, branchCommit, the hash of branchConfig and managerFingerprint to determine if the branch needs to be updated

Context

  • Detect and prevent writing the same updates on re-runs

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 tick 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 draft July 20, 2022 06:21
@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review July 20, 2022 06:37
@RahulGautamSingh

This comment was marked as outdated.

@rarkins
Copy link
Collaborator

rarkins commented Jul 20, 2022

The fingerprint/hash of an update should be the combination of manager fingerprint plus config hash for that update. Then the fingerprint of a branch is a hash of the list of update hashes.

Alternatively you could just use the branch config (which has update configs nested inside) and then combine with the hashes of all managers in the branch.

@RahulGautamSingh RahulGautamSingh requested review from rarkins and viceice and removed request for viceice and rarkins July 20, 2022 23:04
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

needs test fixes

@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Jul 23, 2022

needs test fixes

Updated the tests, they still need some beautification. I will refactor more once the main work is verified.🤞

@RahulGautamSingh RahulGautamSingh marked this pull request as draft August 17, 2022 17:57
@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Aug 17, 2022

I think we should not use commitSha to deicide whether branchFingerprint changes instead skipBranchUpdate value should be used.
Reason: Incase a branch isn't updated but PR is updated , commitSha=null and don't update the cache but it should be updated, based on our current logic where the fingerprint hash is combination branch and PR config.

@rarkins
Copy link
Collaborator

rarkins commented Aug 17, 2022

The branch cache fingerprint should be updated any time the SHA of the branch changes.

@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review August 18, 2022 00:14
@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented Aug 18, 2022

New changes:

  • Sort the branchConfig before generating hash with it, because the order of the config when creating a branch and after it differ which results in different fingerprints for the same config.

Using stringify method from a new package(stable-safe-stringify) instead of using a similar package(fast-safe-stringify) we have because the current one doesn't sort the config even when using the method suggested in docs.

package.json Outdated Show resolved Hide resolved
@rarkins rarkins enabled auto-merge (squash) August 22, 2022 08:50
@rarkins rarkins merged commit 1a25ac6 into renovatebot:main Aug 22, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.171.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2022
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

5 participants