Skip to content

refactor(internal/librarian): dispatch fillDefaults by language instead of type switch - #6624

Closed
JoeWang1127 wants to merge 2 commits into
mainfrom
refactor/fill-default
Closed

refactor(internal/librarian): dispatch fillDefaults by language instead of type switch#6624
JoeWang1127 wants to merge 2 commits into
mainfrom
refactor/fill-default

Conversation

@JoeWang1127

@JoeWang1127 JoeWang1127 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Replace the type switch in fillDefaults that inferred the language from which Default field was non-nil with a languageDefaultFillers map keyed by the explicit language string from Config.Language.

This eliminates the implicit priority ordering between languages (e.g. Java before Rust) and the fragile dispatch based on data rather than the known language identifier.

Each language-specific fill function now guards against its default field being nil, since the type switch no longer prevents calls when the language-specific default is absent.

Motivated by #6554 (comment)

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request refactors the fillDefaults function in internal/librarian/library.go to use a map lookup (languageDefaultFillers) instead of a large switch statement. To prevent nil pointer dereferences, safety checks have been added to each language-specific filler function. Additionally, the unit tests in internal/librarian/library_test.go have been refactored into language-specific test functions and updated to pass the new language parameter. There are no review comments, and I have no feedback to provide.

@JoeWang1127 JoeWang1127 changed the title refactor(internal/librarian): dispatch fillDefaults by language instead of type switch refactor(internal/librarian): dispatch fillDefaults by language instead of type switch Jul 4, 2026
@JoeWang1127
JoeWang1127 marked this pull request as ready for review July 6, 2026 14:27
@JoeWang1127
JoeWang1127 requested a review from a team as a code owner July 6, 2026 14:27
@JoeWang1127
JoeWang1127 requested a review from ianthehat July 6, 2026 14:27
@JoeWang1127

Copy link
Copy Markdown
Contributor Author

Talked with @ianthehat, this change doesn't solve the design issue of config.Config: it ties the public API to a serialize form so they have to change in sync.

@JoeWang1127 JoeWang1127 closed this Jul 6, 2026
@JoeWang1127
JoeWang1127 deleted the refactor/fill-default branch July 6, 2026 15:18
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.

1 participant