Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replacing REDIS_STATIC with static #691

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

ouriamzn
Copy link
Contributor

@ouriamzn ouriamzn commented Jun 25, 2024

As discussed, we want to remove the old REDIS_STATIC flag which is no longer relevant.

When moving from Redis to Valkey we renamed all REDIS flags in Makefile.
The REDIS_STATIC flag was renamed to SERVER_STATIC, but this change was not updated in some of the files.

After discussing it with @madolson and @ranshid, we decided that since this was introduced 10 years ago, and in many places in the code base we simply use static, we should simplify and remove the flag entirely.

@madolson madolson requested a review from ranshid June 25, 2024 15:17
@bbarani
Copy link

bbarani commented Jun 25, 2024

@ouriamzn can you please fix the DCO? It looks like the commit is incorrectly signed off.

@zuiderkwast
Copy link
Contributor

As discussed

That's nothing I've heard about. Who discussed this, when and where?

@madolson
Copy link
Member

@zuiderkwast Previous PR: #684

Copy link
Member

@ranshid ranshid left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM

I think fix the top comment so that this PR stand by itself.

Copy link

codecov bot commented Jun 26, 2024

Codecov Report

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

Project coverage is 70.26%. Comparing base (ce79539) to head (7072fe1).
Report is 5 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #691      +/-   ##
============================================
+ Coverage     70.05%   70.26%   +0.20%     
============================================
  Files           110      110              
  Lines         60084    60104      +20     
============================================
+ Hits          42094    42231     +137     
+ Misses        17990    17873     -117     
Files Coverage Δ
src/quicklist.c 84.72% <93.75%> (ø)

... and 10 files with indirect coverage changes

Signed-off-by: Ouri Half <[email protected]>
@madolson madolson merged commit ab38730 into valkey-io:unstable Jun 26, 2024
19 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.

5 participants