-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
feature: supports yml configuration format. fixes #653 #1863
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1863 +/- ##
=========================================
Coverage ? 48.41%
Complexity ? 1919
=========================================
Files ? 395
Lines ? 11720
Branches ? 1232
=========================================
Hits ? 5674
Misses ? 5596
Partials ? 450
Continue to review full report at Codecov.
|
Looks promising! |
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java
Show resolved
Hide resolved
To support yml, there are 2 possible solutions:
However, I'm not sure which one is better, i.e. easier to implement and easier to extend. |
As I know, using solution 1 makes the key unreadable when the configuration is complex, which is exactly the situation where people prefer yml compared to other formats. The keys look like Solution 2 would be flexible in client side and can be parsed into custom config bean. So I prefer solution 1 ^_^ |
Closing as this approach is not preferable |
I'm not sure which one is better, but this is the next big feature to be done... |
Then I'll keep this PR opened and try another preferable solution(in my opinion), when it's done, we'll compare and pick one of them(if any of them is acceptable) |
I'm working on #1944 to add yml support, which uses the solution |
@nobodyiam will take a look soon(perhaps later tonight or tomorrow) |
This patch tries to fixes the issue #653 , though at this moment this patch only supports simple yml format and doesn't take complex situations into account such as array.
I open this PR in case that anyone is to do the same thing, and, if anyone has any advices after reviewing this commit, he/she can give constructive suggestions here and I'll update this patch.