Skip to content

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

Merged
ahrtr merged 1 commit intoetcd-io:mainfrom
callthingsoff:fix_unique_urls
Jul 26, 2023
Merged

pkg/flags: fix UniqueURLs'Set to remove duplicates in UniqueURLs'uss#16272
ahrtr merged 1 commit intoetcd-io:mainfrom
callthingsoff:fix_unique_urls

Conversation

@callthingsoff
Copy link
Copy Markdown
Contributor

@callthingsoff callthingsoff commented Jul 19, 2023

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.

ahrtr
ahrtr reviewed Jul 21, 2023
View reviewed changes
Comment thread pkg/flags/unique_urls.go
@callthingsoff
Copy link
Copy Markdown
Contributor Author

callthingsoff commented Jul 23, 2023

Looking forward to reply. Thanks.

ahrtr
ahrtr reviewed Jul 23, 2023
View reviewed changes
Comment thread pkg/flags/unique_urls_test.go Outdated
Comment thread pkg/flags/unique_urls_test.go Outdated
ahrtr
ahrtr approved these changes Jul 23, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

callthingsoff commented Jul 26, 2023

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
Copy Markdown
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
Copy Markdown
Member

ahrtr commented Jul 26, 2023

@gocurr Please signoff the commit

@callthingsoff
Copy link
Copy Markdown
Contributor Author

callthingsoff commented Jul 26, 2023

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

callthingsoff commented Jul 26, 2023

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
Copy Markdown
Contributor Author

callthingsoff commented Jul 26, 2023

Sorry, should it be abandoned? Sorry again.

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

callthingsoff commented Jul 26, 2023

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

@ahrtr
Copy link
Copy Markdown
Member

ahrtr commented Jul 26, 2023

Please rebase this PR

ahrtr
ahrtr reviewed Jul 26, 2023
View reviewed changes
Comment thread pkg/flags/unique_urls_test.go Outdated
@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 <xigua67damn@gmail.com>
@callthingsoff
Copy link
Copy Markdown
Contributor Author

callthingsoff commented Jul 26, 2023

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