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

registry type support all #2140

Merged
merged 10 commits into from Dec 11, 2022
Merged

registry type support all #2140

merged 10 commits into from Dec 11, 2022

Conversation

bobtthp
Copy link
Contributor

@bobtthp bobtthp commented Nov 27, 2022

What this PR does:
#2095

Which issue(s) this PR fixes:
Fixes #

You should pay attention to items below to ensure your pr passes our ci test
We do not merge pr with ci tests failed

  • All ut passed (run 'go test ./...' in project root)
  • After go-fmt ed , run 'go fmt project' using goland.
  • Golangci-lint passed, run 'sudo golangci-lint run' in project root.
  • Your new-created file needs to have apache license at the top, like other existed file does.
  • All integration test passed. You can run integration test locally (with docker env). Clone our dubbo-go-samples project and replace the go.mod to your dubbo-go, and run 'sudo sh start_integration_test.sh' at root of samples project root. (M1 Slice is not Support)

@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2022

Codecov Report

Merging #2140 (4ffc0b1) into 3.0 (1cf64b4) will increase coverage by 0.05%.
The diff coverage is 65.00%.

@@            Coverage Diff             @@
##              3.0    #2140      +/-   ##
==========================================
+ Coverage   44.33%   44.39%   +0.05%     
==========================================
  Files         283      283              
  Lines       17082    17082              
==========================================
+ Hits         7574     7584      +10     
+ Misses       8715     8703      -12     
- Partials      793      795       +2     
Impacted Files Coverage Δ
config/service_config.go 53.04% <50.00%> (ø)
config/registry_config.go 81.57% <61.76%> (ø)
config/config_utils.go 100.00% <100.00%> (ø)
registry/protocol/protocol.go 72.10% <100.00%> (ø)
metrics/prometheus/reporter.go 30.25% <0.00%> (-1.54%) ⬇️
metadata/report/delegate/delegate_report.go 35.09% <0.00%> (+8.60%) ⬆️

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

return urls, nil
}

if c.RegistryType == constant.RegistryTypeService {
Copy link
Member

Choose a reason for hiding this comment

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

用switch 也许更好一些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是哦,我来修改下

@justxuewei justxuewei marked this pull request as draft December 4, 2022 02:57
@justxuewei justxuewei marked this pull request as ready for review December 4, 2022 05:55
@bobtthp
Copy link
Contributor Author

bobtthp commented Dec 7, 2022

看了下 samples ci 错误,依赖的 gost 1.13.2 nacos v1 所以失败了

@sonarcloud
Copy link

sonarcloud bot commented Dec 11, 2022

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 AlexStocks merged commit d50f955 into apache:3.0 Dec 11, 2022
Lvnszn pushed a commit to Lvnszn/dubbo-go that referenced this pull request Jan 15, 2023
* registry type support all

* fix test

* set default to interface

* use default protocol registry

* fix unit test

* use swith to judge

* add registry support all test

* Resolve registry name conflicts

* fix ut err

Co-authored-by: bobtthp <bobtthp@bob-Mac-mini.local>
Co-authored-by: bob <bob@bobdeMacBook-Pro.local>
Co-authored-by: bobtthp <bobtthp@bob1.local>
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.

None yet

5 participants