Skip to content

fix(conf): unify datasource constructor return types - #1265

Merged
sysulq merged 1 commit into
douyu:masterfrom
zhouxiaohuan:codex/fix-conf-datasource-return-type
Sep 9, 2026
Merged

sysulq merged 1 commit into
douyu:masterfrom
zhouxiaohuan:codex/fix-conf-datasource-return-type

Conversation

@zhouxiaohuan

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

The file and HTTP configuration datasource constructors return unexported concrete types, while the Apollo and etcdv3 constructors return the public conf.DataSource interface. This PR makes all pkg/conf/datasource NewDataSource constructors expose the same public interface, keeping callers decoupled from implementation details.

Does this pull request fix one issue?

Fixes #1264

Describe how you did it

  • Changed pkg/conf/datasource/file.NewDataSource to return conf.DataSource.
  • Changed pkg/conf/datasource/http.NewDataSource to return conf.DataSource.
  • Updated the two Go doc comments; runtime behavior is unchanged.

Describe how to verify it

  • go test -race ./pkg/conf/...
  • go vet ./pkg/conf/...
  • go build ./...
  • golangci-lint v1.62.2 run --new-from-rev=origin/master
  • git diff --check

The focused checks pass locally. A full go test ./... was also attempted, but this machine does not have the repository's etcd, Redis, RocketMQ, and MySQL services (or Docker) running; the resulting failures are in unrelated integration tests.

Special notes for reviews

The concrete return types are intentionally hidden because the public contract is conf.DataSource; the repository's existing call sites already consume the interface. The PR is based on the current master branch.

@codecov

codecov Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.20%. Comparing base (c41ec70) to head (e5d3611).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1265      +/-   ##
==========================================
+ Coverage   47.09%   51.20%   +4.11%     
==========================================
  Files         236      236              
  Lines       10921     9200    -1721     
==========================================
- Hits         5143     4711     -432     
+ Misses       5326     4489     -837     
+ Partials      452        0     -452     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zhouxiaohuan

Copy link
Copy Markdown
Contributor Author

CI note: the golangci-lint workflow currently reports pre-existing unused symbols in pkg/server/xgin/middleware.go and pkg/server/xgrpc/interceptor.go; the same baseline failures are present in recent upstream PRs #1262 and #1263. The changed datasource files have no findings under golangci-lint v1.62.2 locally. All other PR checks, including unit-test, E2E, build, CodeQL, docs, and Codecov, are passing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes are small, consistent with the stated goal, and preserve behavior while aligning constructor return types to the existing conf.DataSource interface.

Pull request overview

This PR standardizes the public constructor API across pkg/conf/datasource implementations by having the file- and HTTP-backed NewDataSource constructors return the public conf.DataSource interface (matching the Apollo and etcdv3 implementations), keeping callers decoupled from concrete datasource types.

Changes:

  • Updated pkg/conf/datasource/file.NewDataSource to return conf.DataSource.
  • Updated pkg/conf/datasource/http.NewDataSource to return conf.DataSource.
  • Refined Go doc comments on both constructors to describe the backing implementation (file/HTTP).
File summaries
File Description
pkg/conf/datasource/http/yasee.go Returns conf.DataSource from the HTTP datasource constructor and updates its exported doc comment.
pkg/conf/datasource/file/file.go Returns conf.DataSource from the file datasource constructor and updates its exported doc comment.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@sysulq
sysulq merged commit d43b770 into douyu:master Sep 9, 2026
10 of 13 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.

fix(conf): return conf.DataSource from all datasource constructors

3 participants