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

fix:change keysCommand default format auto #597

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

Elbehery
Copy link
Member

@Elbehery Elbehery commented Nov 6, 2023

This PR changes keysCommand default format from ascii-encoded to auto

resolves ##588

cc @ahrtr

key := fmt.Sprintf("%s-%d", name, i)
if err := b.Put([]byte(key), []byte{0}); err != nil {
for _, k := range []int64{10001, 10002} {
if err := b.Put(convertInt64IntoBytes(t, k), []byte{0}); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
if err := b.Put(convertInt64IntoBytes(t, k), []byte{0}); err != nil {
if err := b.Put(convertInt64IntoBytes(t, k), []byte{0}); err != nil {

convertInt64IntoBytes is being picked at #596

cc @ahrtr

@Elbehery Elbehery force-pushed the keysCommand_default_format_auto branch 3 times, most recently from 0d8163d to d641cad Compare November 7, 2023 17:14
@Elbehery
Copy link
Member Author

Elbehery commented Nov 7, 2023

@ahrtr I updated this accordingly, ptal 👍🏽

@Elbehery Elbehery force-pushed the keysCommand_default_format_auto branch from d641cad to 337eafd Compare November 7, 2023 17:18
@Elbehery
Copy link
Member Author

Elbehery commented Nov 8, 2023

/assign @ahrtr

@Elbehery
Copy link
Member Author

Elbehery commented Nov 8, 2023

@fuweid would appreciate your review :)

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -677,3 +709,11 @@ func convertInt64IntoBytes(num int64) []byte {
n := binary.PutVarint(buf, num)
return buf[:n]
}

func convertInt64IntoBytesString(nums ...int64) string {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func convertInt64IntoBytesString(nums ...int64) string {
func convertInt64KeysIntoHexString(nums ...int64) string {

t.Logf("creating test bucket %s", tc.testBucket)
b, bErr := tx.CreateBucketIfNotExists([]byte(tc.testBucket))
if bErr != nil {
return fmt.Errorf("error creating test bucket '%s': '%v'", tc.testBucket, bErr)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("error creating test bucket '%s': '%v'", tc.testBucket, bErr)
return fmt.Errorf("error creating test bucket %q: %v", tc.testBucket, bErr)

@ahrtr
Copy link
Member

ahrtr commented Nov 8, 2023

Please also rebase this PR.

Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
@Elbehery Elbehery force-pushed the keysCommand_default_format_auto branch from 337eafd to 09dd42f Compare November 8, 2023 10:20
@Elbehery
Copy link
Member Author

Elbehery commented Nov 8, 2023

@ahrtr done 👍🏽

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr
Copy link
Member

ahrtr commented Nov 8, 2023

Please update the readme in a followup PR.

@ahrtr ahrtr merged commit 76ae8f2 into etcd-io:master Nov 8, 2023
9 checks passed
@Elbehery
Copy link
Member Author

Elbehery commented Nov 8, 2023

resolves ##588

@ahrtr raised #604

@Elbehery Elbehery deleted the keysCommand_default_format_auto branch January 12, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants