refactor: eliminate small-package smell#85
Merged
Merged
Conversation
- Delete internal/httputil: inline writeJSON/writeError as unexported helpers in cmd/proxy (httputil.go) and internal/server (httputil.go) - Delete internal/metrics: fold HTTPRequestsTotal, HTTPRequestDuration, and Register() into internal/server as unexported vars + RegisterMetrics() - Move internal/inference → cmd/proxy/inference/ (proxy-only demo code; no other package used it) - Split internal/config into internal/charonconfig (CharonOptions + ByteSize) and internal/proxyconfig (ProxyOptions); each holds its own private YAML loader and TelemetryOptions; charon.listen is passed as a plain string to proxyconfig so there is no cross-package dependency Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Replace bare 'charon' (which also matched cmd/charon/) with rooted /charon and /proxy so only the top-level binaries are excluded and cmd/* source directories are tracked normally. Signed-off-by: Etai Lev Ran <elevran@gmail.com>
elevran
commented
Jul 8, 2026
elevran
commented
Jul 8, 2026
- Delete internal/httputil: inline writeJSON/writeError as unexported helpers in cmd/proxy and internal/server (WriteJSON/WriteError exported from internal/server so cmd/proxy can use them without a separate pkg) - Delete internal/metrics: fold HTTPRequestsTotal, HTTPRequestDuration, and Register() into internal/server as unexported vars + RegisterMetrics() - Move TelemetryOptions to internal/telemetry as Options; both charonconfig and proxyconfig import it instead of defining their own copy - Split internal/config into internal/charonconfig (CharonOptions + ByteSize) and internal/proxyconfig (ProxyOptions); each has its own private YAML loader with strict unmarshalling; no shared config file between the two processes — all json.RawMessage absorbers removed - Simplify proxyconfig: collapse fileConfigRaw+fileConfig into a single fileConfig struct; fileCharonSection holds only Listen (no storage/workers absorbers); applyFileDefaults called post-unmarshal only - Fix .gitignore: build outputs go under ./bin/; remove stale /charon and /proxy entries that blocked staging source files - Update Makefile: build targets write to ./bin/; add clean target Signed-off-by: Etai Lev Ran <elevran@gmail.com>
Remove all testdata/*.yaml files from charonconfig and proxyconfig. Replace file references with inline []byte literals written to temp files via a configFromBytes helper. Expand strict-parse rejection tests into table-driven subtests covering unknown keys at each nesting level. Signed-off-by: Etai Lev Ran <elevran@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
internal/httputil— inlinewriteJSON/writeErroras unexported helpers directly incmd/proxyandinternal/server; no shared abstraction needed for two call sitesinternal/metrics— fold the two Prometheus vars andRegister()intointernal/serverasRegisterMetrics(); onlyinternal/server/middleware.goand the two entry points ever touched itinternal/inference→cmd/proxy/inference/— demo/proxy-only code with no consumers outsidecmd/proxy; keeping it underinternal/implied broader reuse that doesn't existinternal/config→internal/charonconfig+internal/proxyconfig—CharonOptionsandByteSizelive incharonconfig;ProxyOptionsinproxyconfig; each owns its own private YAML loader andTelemetryOptions; charon's listen address is passed as a plain string so there is no cross-package dependency.gitignore— replace barecharon(which also excludedcmd/charon/source) with rooted/charonand/proxyAll packages build clean and
make presubmitpasses.Test plan
make presubmitpasses (fmt + vet + tests)cmd/proxy/inferencetests pass independentlyinternal/charonconfigandinternal/proxyconfigtests cover defaults, file loading, CLI override, and strict-parse rejection