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.

Sign up for GitHub

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

Jump to bottom

pkg/flags: fix UniqueURLs'Set to remove duplicates in UniqueURLs'uss #16272

Merged
merged 1 commit into from
Jul 26, 2023
Merged

pkg/flags: fix UniqueURLs'Set to remove duplicates in UniqueURLs'uss #16272

merged 1 commit into from
Jul 26, 2023

Conversation

callthingsoff
Copy link
Contributor

From the name of func 'UniqueURLsFromFlag', we can tell that UniqueURLs'uss should not have duplicates. The current implemention of UniqueURLs'Set has a bug to make it unique. This PR fixes it.

Please take a look.

@callthingsoff
Copy link
Contributor Author

Looking forward to reply. Thanks.

ahrtr
ahrtr reviewed Jul 23, 2023
View reviewed changes
pkg/flags/unique_urls_test.go Outdated Show resolved Hide resolved
pkg/flags/unique_urls_test.go Outdated Show resolved Hide resolved
ahrtr
ahrtr approved these changes Jul 23, 2023
View reviewed changes
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

thx @gocurr

Please backport this PR to both 3.5 and 3.4 if needed.

fuweid
fuweid approved these changes Jul 24, 2023
View reviewed changes
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

Thank you for explanation and patient.

@callthingsoff
Copy link
Contributor Author

LGTM

thx @gocurr

Please backport this PR to both 3.5 and 3.4 if needed.

For now? Or after this is merged(if accepted)?

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2023

This PR needs to be backported to both 3.5 and 3.4. Please raise an issue to track the effort. Otherwise it's hard to track the task after merging this PR.

@callthingsoff callthingsoff mentioned this pull request Jul 26, 2023
Closed
@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2023

@gocurr Please signoff the commit

@callthingsoff
Copy link
Contributor Author

Sorry, I don't see anything wrong with the commit message.
It ends with Signed-off-by:, Could you tell me what's wrong?

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2023

Please check:

  • If the email is bound to your github account;
  • If the email after Signed-off-by matches the email included in Author of the commit.

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2023

See https://github.com/etcd-io/etcd/pull/16272/checks?check_run_id=15358965509

Screenshot 2023-07-26 at 13 27 58

@callthingsoff
Copy link
Contributor Author

Maybe it's the #16308, this pull uses wrong branch, and I closed it.
I don't know how to handle this.

@callthingsoff callthingsoff mentioned this pull request Jul 26, 2023
Merged
@callthingsoff
Copy link
Contributor Author

Sorry, should it be abandoned? Sorry again.

@callthingsoff callthingsoff mentioned this pull request Jul 26, 2023
Merged
@callthingsoff
Copy link
Contributor Author

Sorry, I'm not very familiar with github's workflow yet.
If there are any questions, please let me know.

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2023

Please rebase this PR

@callthingsoff
pkg/flags: fix UniqueURLs'Set to remove duplicates in UniqueURLs'uss
4f78cc0 
From the name of func 'UniqueURLsFromFlag', we can tell that UniqueURLs'uss
should not have duplicates. The current implemention of UniqueURLs'Set
has a bug to make it unique.

Fixes: #16307.

Signed-off-by: Jes Cok <[email protected]>
@callthingsoff
Copy link
Contributor Author

How I wish it can be cherry-picked.
In fact I really enjoy Gerrit Code Review.

@ahrtr ahrtr merged commit af07f18 into etcd-io:main Jul 26, 2023
@callthingsoff callthingsoff deleted the fix_unique_urls branch July 26, 2023 17:59
ahrtr added a commit that referenced this pull request Jul 27, 2023
@ahrtr
Merge pull request #16311 from gocurr/release-3.5
b924891 
[3.5] Backport #16272 to 3.5
ahrtr added a commit that referenced this pull request Jul 27, 2023
@ahrtr
Merge pull request #16309 from gocurr/release-3.4
e8c6c93 
[3.4] Backport #16272 to 3.4
@serathius serathius mentioned this pull request Oct 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahrtr ahrtr ahrtr approved these changes

@fuweid fuweid fuweid approved these changes

Assignees
No one assigned
Labels
backport/v3.4 backport/v3.5
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@callthingsoff @ahrtr @fuweid