-
Notifications
You must be signed in to change notification settings - Fork 43
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
Remove ProviderBuilder #3282
Remove ProviderBuilder #3282
Conversation
Fixes #2845 Remove final usages of provider builder in CLI and delete code.
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.
✅ No Invisible Unicode Characters Detected.
return manager.NewProviderManager(providerStore, githubProviderManager) | ||
} | ||
|
||
func wireUpDB(ctx context.Context, cfg *serverconfig.Config) (db.Store, error) { |
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.
I refactored some of this code into separate functions out to reduce cyclomatic complexity of the runCmdWebhookUpdate
(the new code I added exceeded the cyclomatic complexity limit)
continue | ||
} | ||
|
||
whSecret, err := cfg.WebhookConfig.GetWebhookSecret() |
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.
As far as I can tell, this is constant across all iterations of the loop, so I moved it outside of the loop
continue | ||
} | ||
|
||
if whSecret == "" { |
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.
If this is empty, no webhooks are updated, so I converted to an error condition (see line 263)
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.
✅ No Invisible Unicode Characters Detected.
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.
Fixes #2845
Remove final usages of provider builder in CLI and delete code.
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: