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

Add aliases for masteruser and masterauth to primaryuser and primaryauth #598

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

madolson
Copy link
Member

@madolson madolson commented Jun 4, 2024

Make the one backwards compatible config change we are allowed to replace for removing master from our API.

masterauth and masteruser are still used as an alias, but aren't explicitly referenced. As an addendum to #591, it would be good to have this in 8. Given the related PR for updated other references for master, I just updated the ones around this specific change.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@madolson madolson added release-notes This issue should get a line item in the release notes major-decision-pending Major decision pending by TSC team de-crapify Correct crap decisions made in the past labels Jun 4, 2024
Copy link

codecov bot commented Jun 4, 2024

Codecov Report

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

Project coverage is 70.19%. Comparing base (0700c44) to head (ef246bb).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #598      +/-   ##
============================================
+ Coverage     70.07%   70.19%   +0.11%     
============================================
  Files           110      110              
  Lines         59991    59992       +1     
============================================
+ Hits          42038    42110      +72     
+ Misses        17953    17882      -71     
Files Coverage Δ
src/config.c 78.31% <ø> (ø)
src/valkey-cli.c 53.25% <0.00%> (+1.67%) ⬆️

... and 8 files with indirect coverage changes

Copy link
Contributor

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

I think we should leave/add a test with masteruser/masterauth for test sanity of alias logic.

@madolson
Copy link
Member Author

madolson commented Jun 4, 2024

I think we should leave/add a test with masteruser/masterauth for test sanity of alias logic.

Sure.

@PingXie PingXie merged commit bce240e into valkey-io:unstable Jun 7, 2024
17 checks passed
@madolson madolson added major-decision-approved Major decision approved by TSC team and removed major-decision-pending Major decision pending by TSC team labels Jun 7, 2024
advaMosh pushed a commit to advaMosh/valkey that referenced this pull request Jun 10, 2024
…alkey-io#598)

Make the one backwards compatible config change we are allowed to
replace for removing master from our API.

`masterauth` and `masteruser` are still used as an alias, but aren't
explicitly referenced. As an addendum to
valkey-io#591, it would be good to have
this in 8. Given the related PR for updated other references for master,
I just updated the ones around this specific change.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Adva Moshkovitz <advamo7@gmail.com>
naglera pushed a commit to naglera/placeholderkv that referenced this pull request Jun 10, 2024
…alkey-io#598)

Make the one backwards compatible config change we are allowed to
replace for removing master from our API.

`masterauth` and `masteruser` are still used as an alias, but aren't
explicitly referenced. As an addendum to
valkey-io#591, it would be good to have
this in 8. Given the related PR for updated other references for master,
I just updated the ones around this specific change.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@madolson madolson changed the title Replace masteruser and masterauth with primaryuser and primaryauth Add aliases for masteruser and masterauth to primaryuser and primaryauth Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
de-crapify Correct crap decisions made in the past major-decision-approved Major decision approved by TSC team release-notes This issue should get a line item in the release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants