Skip to content

refactor(core): replace TakeOne usage with cmp.Or#5461

Merged
kevwan merged 1 commit intozeromicro:masterfrom
1911860538:core/stringxor
Mar 22, 2026
Merged

refactor(core): replace TakeOne usage with cmp.Or#5461
kevwan merged 1 commit intozeromicro:masterfrom
1911860538:core/stringxor

Conversation

@1911860538
Copy link
Contributor

The custom TakeOne function is redundant since Go 1.21 introduced cmp.Or,
which provides identical functionality for selecting the first non-zero value.

Changes:

  • Mark TakeOne as deprecated with a pointer to cmp.Or.
  • Replace all internal usages of TakeOne with cmp.Or.

Copy link
Contributor

@kevwan kevwan left a comment

Choose a reason for hiding this comment

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

Review

Clean, idiomatic Go modernisation. cmp.Or (available since Go 1.21, project requires Go 1.23) is the canonical stdlib replacement for the "first non-zero" pattern — removing a dependency on the internal stringx package from core/mapping is a clear improvement.

core/mapping/utils.go — Both stringx.TakeOne(cache.key, field.Name)cmp.Or(cache.key, field.Name) replacements are correct. The semantics are identical for strings: returns the first non-empty value, falling back to the second.

core/stringx/strings.go — Adding // Deprecated: use cmp.Or instead. to TakeOne is the right signal. One small suggestion: follow Go's conventional deprecation comment format so tooling picks it up:

// Deprecated: Use cmp.Or from the standard library instead.
func TakeOne(valid, or string) string {

(The conventional form starts with Deprecated: on a line of its own or as a sentence within the godoc comment.)

Overall: LGTM. The removal of the internal import simplifies the dependency graph in core/mapping.

@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kevwan kevwan added this pull request to the merge queue Mar 22, 2026
Merged via the queue into zeromicro:master with commit 8cd7f7a Mar 22, 2026
6 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