Skip to content

Ensure PoolSize is set in RepoManager#167

Merged
yushan8 merged 4 commits into
mainfrom
yushan/error-poolsize
Jul 9, 2026
Merged

Ensure PoolSize is set in RepoManager#167
yushan8 merged 4 commits into
mainfrom
yushan/error-poolsize

Conversation

@yushan8

@yushan8 yushan8 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

PoolSize is a required config field. Error out in the constructor if not set.

@yushan8 yushan8 requested review from a team as code owners July 9, 2026 20:04

@sywhang sywhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add test?

@sywhang sywhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add test?

// shutting down (e.g. wire it to SIGTERM/SIGINT in main) to abort any
// background goroutines the manager spawns.
func NewRepoManager(appCtx context.Context, p Params) RepoManager {
func NewRepoManager(appCtx context.Context, p Params) (RepoManager, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually, since this signature's changed, all the callers have to be updated too.

Comment thread core/repomanager/repo_manager_test.go Outdated
Comment thread core/repomanager/repo_manager_test.go Outdated
Comment thread core/repomanager/repo_manager_test.go Outdated
@yushan8 yushan8 merged commit 450d404 into main Jul 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants