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:support file extension #1626

Merged
merged 1 commit into from
Dec 2, 2021
Merged

add:support file extension #1626

merged 1 commit into from
Dec 2, 2021

Conversation

zhaoyunxing92
Copy link
Contributor

What this PR does:
配置中心支持自定义文件类型解析器,目前支持jsonyamlpropertiesymltoml五种文件类型配置
Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@zhaoyunxing92 zhaoyunxing92 added this to the v3.0.0 milestone Nov 30, 2021
@zhaoyunxing92 zhaoyunxing92 changed the title add:support file extension [WIP]add:support file extension Nov 30, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #1626 (e8b58bf) into 3.0 (a7bbf32) will increase coverage by 0.11%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.0    #1626      +/-   ##
==========================================
+ Coverage   40.94%   41.06%   +0.11%     
==========================================
  Files         252      252              
  Lines       14443    14445       +2     
==========================================
+ Hits         5914     5932      +18     
+ Misses       7848     7830      -18     
- Partials      681      683       +2     
Impacted Files Coverage Δ
config/config_center_config.go 50.00% <0.00%> (+0.96%) ⬆️
config/config_loader_options.go 61.97% <100.00%> (+8.24%) ⬆️
cluster/cluster/failback/cluster_invoker.go 75.82% <0.00%> (-2.20%) ⬇️
metrics/prometheus/reporter.go 26.85% <0.00%> (-1.72%) ⬇️
metadata/report/delegate/delegate_report.go 34.43% <0.00%> (+7.94%) ⬆️
filter/metrics/filter.go 100.00% <0.00%> (+15.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7bbf32...e8b58bf. Read the comment docs.

@LaurenceLiZhixin
Copy link
Contributor

记得在samples 仓库里给出对应例子。 @zhaoyunxing92

@@ -155,11 +161,11 @@ func userHomeDir() string {

// checkGenre check Genre
func checkGenre(genre string) error {
genres := []string{"json", "toml", "yaml", "yml"}
genres := []string{"json", "toml", "yaml", "yml", "properties"}
sort.Strings(genres)
idx := sort.SearchStrings(genres, genre)
if genres[idx] != genre {
Copy link
Member

Choose a reason for hiding this comment

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

这里的判断是不是需要判断边界情况 idx == len(genres) || genres[idx] != genre

Copy link
Member

Choose a reason for hiding this comment

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

+1,idx 有没有可能是 -1 的情况?

Copy link
Contributor Author

@zhaoyunxing92 zhaoyunxing92 Dec 1, 2021

Choose a reason for hiding this comment

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

源码里面就是一个二分搜索,不会存在-1的情况

func Search(n int, f func(int) bool) int {
	// Define f(-1) == false and f(n) == true.
	// Invariant: f(i-1) == false, f(j) == true.
	i, j := 0, n
	for i < j {
		h := int(uint(i+j) >> 1) // avoid overflow when computing h
		// i ≤ h < j
		if !f(h) {
			i = h + 1 // preserves f(i-1) == false
		} else {
			j = h // preserves f(j) == true
		}
	}
	// i == j, f(i-1) == false, and f(j) (= f(i)) == true  =>  answer is i.
	return i
}

@AlexStocks
Copy link
Contributor

这个 pr 我看标识是 WIP,为啥都有两个 approve 了?

@LaurenceLiZhixin
Copy link
Contributor

这个 pr 我看标识是 WIP,为啥都有两个 approve 了?

我的锅,之前没注意到

@zhaoyunxing92 zhaoyunxing92 changed the title [WIP]add:support file extension add:support file extension Dec 1, 2021
@LaurenceLiZhixin LaurenceLiZhixin merged commit f7f1581 into apache:3.0 Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants