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

chore: Refactor retries in gitlab set status #5293

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukemassa
Copy link
Contributor

@lukemassa lukemassa commented Feb 2, 2025

what

Refactor the way retries work in UpdateStatus in gitlab.

why

I noticed this while working on #5269; we can't prove that by the time we get to "return err" we'll actually have a non-nil error to wrap. In practice this won't happen, if you carefully follow the paths (and note that StatusConflict will cause err to be non-nil), but it's hard to see. Additionally there were comments like "if we get here, then..." and "return the error, which might be nil", which to me is a code smell that the logic is hard to follow.

Instead I refactored following the example of the retry library we were using https://pkg.go.dev/github.com/jpillora/backoff

Another oddity that took me a bit to understand was that, whether there was a conflict or not, we retry if there's an error and we haven't hit our max amount. This means that the conflict really just amounts to a change in logging, which the new code reflects.

tests

The unit tests have good coverage of this area of the code.

references

@lukemassa lukemassa requested review from a team as code owners February 2, 2025 22:45
@lukemassa lukemassa requested review from GenPage, nitrocode and X-Guardian and removed request for a team February 2, 2025 22:45
@dosubot dosubot bot added the refactoring Code refactoring that doesn't add additional functionality label Feb 2, 2025
@github-actions github-actions bot added go Pull requests that update Go code provider/gitlab labels Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code provider/gitlab refactoring Code refactoring that doesn't add additional functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant