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 subscribe any value #2267

Merged
merged 7 commits into from
Apr 18, 2023
Merged

add subscribe any value #2267

merged 7 commits into from
Apr 18, 2023

Conversation

wudong5
Copy link
Contributor

@wudong5 wudong5 commented Mar 26, 2023

What this PR does:

  • Implement the registry.Subscribe interface support subscribing to any value.
  • Try to fix the subscribe bug.

Which issue(s) this PR fixes:

Fixes #2236

Signed-off-by: wudong ustbwmd@163.com

@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@justxuewei
Copy link
Member

Before #2262 is resolved, please merge the code into "3.0" branch.

@chickenlj
Copy link
Contributor

Before #2262 is resolved, please merge the code into "3.0" branch.

To make sure 3.0 branch always represents the latest stable version, we must restrict only bugfixes can be merged into it to avoid any possible unexpected changes.

I suggest merging this patch into the 3.1 feature branch. Let's wait for the branch adjustment to finish first.

@chickenlj chickenlj changed the base branch from 3.1 to main March 28, 2023 12:55
@justxuewei
Copy link
Member

Please pull the latest commits after #2280 is merged to address the CI issues.

@wudong5 wudong5 closed this Mar 30, 2023
@wudong5 wudong5 reopened this Mar 30, 2023
@wudong5
Copy link
Contributor Author

wudong5 commented Mar 30, 2023

Before #2262 is resolved, please merge the code into "3.0" branch.

done

@justxuewei
Copy link
Member

justxuewei commented Apr 3, 2023

Please follow our format of pull request message: give a brief introduction of what this PR does, and what a issue this PR fixes, see: #2280.

@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2023

Codecov Report

Merging #2267 (3fcc135) into main (3fcc135) will not change coverage.
The diff coverage is n/a.

❗ Current head 3fcc135 differs from pull request most recent head 797201b. Consider uploading reports for the commit 797201b to get more accurate results

@@           Coverage Diff           @@
##             main    #2267   +/-   ##
=======================================
  Coverage   44.27%   44.27%           
=======================================
  Files         289      289           
  Lines       18199    18199           
=======================================
  Hits         8058     8058           
  Misses       9293     9293           
  Partials      848      848           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Great works! But a few comments.

common/url.go Outdated Show resolved Hide resolved
registry/zookeeper/listener.go Show resolved Hide resolved
remoting/zookeeper/listener.go Outdated Show resolved Hide resolved
remoting/zookeeper/listener.go Outdated Show resolved Hide resolved
remoting/zookeeper/listener.go Show resolved Hide resolved
remoting/zookeeper/listener.go Outdated Show resolved Hide resolved
remoting/zookeeper/listener.go Outdated Show resolved Hide resolved
remoting/zookeeper/listener.go Show resolved Hide resolved
remoting/zookeeper/listener.go Outdated Show resolved Hide resolved
remoting/zookeeper/listener.go Show resolved Hide resolved
@justxuewei
Copy link
Member

justxuewei commented Apr 3, 2023

Please link an issue that this PR fixes. If there is no issue for this PR, please file one. So the commit message and PR message look like:

Add subscribe any value.

Fixes: #2210

Signed-off-by: Example <example@apache.org>

Btw, the signed-off message could be generated by git with -s option, like: git commit -s. If you want to amend your commit, you can exec git commit -s --amend.

common/url.go Outdated Show resolved Hide resolved
Signed-off-by: wudong <ustbwmd@163.com>
Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@justxuewei
Copy link
Member

Please rebase your PR onto the latest main branch to address the CI failure.

@sonarcloud
Copy link

sonarcloud bot commented Apr 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@AlexStocks
Copy link
Contributor

@wudong5 pls fix the ci failure.

Copy link
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

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

LGTM.

@chickenlj chickenlj merged commit 0ea6075 into apache:main Apr 18, 2023
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.

registry.Subscribe 不支持模糊匹配
5 participants