Skip to content

Cleanup config code#1729

Merged
dgageot merged 4 commits intodocker:mainfrom
dgageot:cleanup-config
Feb 14, 2026
Merged

Cleanup config code#1729
dgageot merged 4 commits intodocker:mainfrom
dgageot:cleanup-config

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Feb 14, 2026

No description provided.

@dgageot dgageot requested a review from a team as a code owner February 14, 2026 09:07
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Code Review Summary

This PR performs a clean refactoring of the config package code:

Resource Management Improvement: Added defer fs.Close() in pkg/config/sources.go to properly close filesystem handles - excellent catch for preventing resource leaks.

Code Organization: Moving GetUserSettings() from pkg/config to pkg/userconfig as Get() improves package boundaries and cohesion.

Refactoring Quality: The extraction of resolveOne(), resolveDirectory(), and singleSource() helper functions makes the resolution logic more modular and easier to understand.

Simplified Code: The new BuiltinAgentNames() implementation using slices.Sorted(maps.Keys(...)) is cleaner and adds consistent ordering.

Functional Equivalence: All logic changes maintain the same behavior as the original code - URL resolution, OCI handling, and alias resolution all work identically.

No issues found. The code is well-structured and maintains backward compatibility.

Signed-off-by: David Gageot <david.gageot@docker.com>
The function only wraps userconfig.Load() + cfg.GetSettings(), so it belongs
in pkg/userconfig rather than pkg/config which is about agent configuration
resolution. Renamed to Get() since the package name already provides context.

Assisted-By: cagent
Load() now accepts Source directly since every caller already passes a Source.
The test helper testfileSource is replaced with the existing NewFileSource().

Assisted-By: cagent
@dgageot dgageot merged commit 15ea9b4 into docker:main Feb 14, 2026
5 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