-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[INS-249] Updated Gitlab client from v0.129.0 to v1.12.0(latest) #4655
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
base: main
Are you sure you want to change the base?
[INS-249] Updated Gitlab client from v0.129.0 to v1.12.0(latest) #4655
Conversation
|
Please do not merge this before this one: #4608 I want these changes to go to Enterprise first. |
kashifkhan0771
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to resolve the conflicts.
Thank you for waiting. The PR is merged, you can resolve the conflicts and we can review this PR. |
Removed unused 'time' import from gitlab.go
camgunz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG! Just want that line split up then merge away 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not create Gitlab OAUTH client for %q: %w", s.url, err) | ||
| return nil, fmt.Errorf("could not create Gitlab TOKEN client for %q: %w", s.url, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect error message for OAUTH authentication case
Low Severity
The error message in the case "OAUTH": block incorrectly says "could not create Gitlab TOKEN client" instead of "could not create Gitlab OAUTH client". This mismatch between the authentication method being used and the error message could cause confusion when debugging authentication failures.
| return nil, fmt.Errorf("could not create Gitlab TOKEN client for %q: %w", s.url, err) | ||
| } | ||
| return apiClient, nil | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated code between OAUTH and TOKEN cases
Low Severity
The case "OAUTH": block (lines 475-486) and case "TOKEN": block (lines 505-516) now have identical implementations after the library upgrade. Both create an oauth2.StaticTokenSource, call gitlab.NewAuthSourceClient with gitlab.OAuthTokenSource{TokenSource: ts}, and use the same options. This duplication increases maintenance burden since any fix or change would need to be applied in both places. These cases could be combined into case "OAUTH", "TOKEN": to eliminate the redundancy.
Description:
This pull request introduces the following changes:
v0.129.0tov1.12.0in go.mod.filesystem_integration_test.goandgitlab_integration_test.godue to missing import ofdefaultspackage.gitlab_integration_test.goto make sure that the test passes.basic auth, scoped repotest to use creds of non-2fa account as gitlab no longer allows repo to be cloned with password of a 2fa enabled account.Checklist:
make test-community)?make lintthis requires golangci-lint)?