Skip to content

Change redis to server in INFO fields redis_{git_sha1,git_dirty,build_id}#1736

Closed
zuiderkwast wants to merge 2 commits into
valkey-io:unstablefrom
zuiderkwast:server-git-info-fields
Closed

Change redis to server in INFO fields redis_{git_sha1,git_dirty,build_id}#1736
zuiderkwast wants to merge 2 commits into
valkey-io:unstablefrom
zuiderkwast:server-git-info-fields

Conversation

@zuiderkwast

Copy link
Copy Markdown
Contributor

These fields were mentioned in #41 but we didn't change them. I don't remember why.

I hope we can changing them in 9.0 when we're phasing out the redis compatibility fields.

… and redis_build_id

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast added the rebranding Valkey is not Redis label Feb 14, 2025
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@codecov

codecov Bot commented Feb 14, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.10%. Comparing base (c5ae37f) to head (07d3ea9).
Report is 56 commits behind head on unstable.

Files with missing lines Patch % Lines
src/release.c 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1736      +/-   ##
============================================
- Coverage     71.17%   71.10%   -0.08%     
============================================
  Files           123      123              
  Lines         65536    65536              
============================================
- Hits          46645    46599      -46     
- Misses        18891    18937      +46     
Files with missing lines Coverage Δ
src/rdma.c 100.00% <ø> (ø)
src/server.c 87.54% <ø> (-0.03%) ⬇️
src/tls.c 100.00% <ø> (ø)
src/release.c 81.25% <75.00%> (ø)

... and 11 files with indirect coverage changes

🚀 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.

Comment thread src/server.c
"redis_git_sha1:%s\r\n", serverGitSHA1(),
"redis_git_dirty:%i\r\n", strtol(serverGitDirty(), NULL, 10) > 0,
"redis_build_id:%s\r\n", serverBuildIdString(),
"server_git_sha1:%s\r\n", serverGitSHA1(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some part of me says let's just leave them. One more thing to not list as a breaking change.

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.

I remember we should keep this info message as is, otherwise is a break change. For the other update in this PR, everything is fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's a breaking change.

So I think we will not change it in 9.0 then. But I'm wondering: How long are we going to keep everything 'redis'? Forever? Five years? If we keep it forever, then users will always remember that Redis was the original and Valkey is just a fork.

If we want to accept the risk, we are allowed to make breaking changes in a major version. I understand "redis_version" is used by many clients, but the git fields are likely not used by many. If we decide to change it some time, I hope users will understand that we want to phase out 'redis' named fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rebranding Valkey is not Redis

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants