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.

Sign up for GitHub

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

Jump to bottom

Split ConfigOption read into two steps: parse() and convert() #40

Merged
merged 5 commits into from
Dec 31, 2019
Merged

Split ConfigOption read into two steps: parse() and convert() #40

merged 5 commits into from
Dec 31, 2019

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Dec 1, 2019

also fix ConfigConvOption/ConfigListConvOption read bug

fix: github.com/hugegraph/hugegraph/issues/774
Change-Id: I716c9f187128be9b0d173f152da05e5bdc2208a3

@codecov
Copy link

codecov bot commented Dec 1, 2019 •
edited
Loading

Codecov Report

Merging #40 into master will increase coverage by 2.39%.
The diff coverage is 79.06%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #40      +/-   ##
===========================================
+ Coverage        78%   80.4%   +2.39%     
- Complexity      584     606      +22     
===========================================
  Files            55      55              
  Lines          1869    1883      +14     
  Branches        275     281       +6     
===========================================
+ Hits           1458    1514      +56     
+ Misses          286     253      -33     
+ Partials        125     116       -9
Impacted Files Coverage Δ Complexity Δ
...ava/com/baidu/hugegraph/version/CommonVersion.java 50% <ø> (ø) 1 <0> (ø) ⬇️
...in/java/com/baidu/hugegraph/config/HugeConfig.java 84.74% <100%> (+7.47%) 17 <0> (+1) ⬆️
...n/java/com/baidu/hugegraph/config/OptionSpace.java 91.3% <100%> (+40.08%) 11 <1> (+6) ⬆️
.../java/com/baidu/hugegraph/config/OptionHolder.java 100% <100%> (+23.07%) 6 <0> (+1) ⬆️
...a/com/baidu/hugegraph/config/ConfigConvOption.java 100% <100%> (+25%) 3 <2> (-1) ⬇️
...a/com/baidu/hugegraph/config/ConfigListOption.java 72.72% <60%> (-8.23%) 6 <2> (ø)
...n/java/com/baidu/hugegraph/config/TypedOption.java 85.45% <66.66%> (+8.71%) 19 <3> (+7) ⬆️
...m/baidu/hugegraph/config/ConfigListConvOption.java 89.47% <81.81%> (+7.12%) 7 <5> (+3) ⬆️
.../main/java/com/baidu/hugegraph/event/EventHub.java 80.55% <0%> (-1.39%) 26% <0%> (-1%)
... and 9 more

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 49bff5e...585adb6. Read the comment docs.

@javeme javeme force-pushed the ConfigConvOption-bug branch from 0d50902 to bd35c4d Compare December 14, 2019 16:21
@javeme
Split ConfigOption read into two steps: parse() and convert()
a6fe4c3 
also fix ConfigConvOption/ConfigListConvOption read bug

fix: github.com/hugegraph/hugegraph/issues/774
Change-Id: I716c9f187128be9b0d173f152da05e5bdc2208a3
@javeme javeme force-pushed the ConfigConvOption-bug branch 2 times, most recently from 69a4411 to 56470e3 Compare December 21, 2019 18:28
@javeme
improve test
748b46f 
Change-Id: Id7aed5ed610d310a901b306e1c05f65074190c8d
@javeme javeme force-pushed the ConfigConvOption-bug branch from 56470e3 to 748b46f Compare December 21, 2019 19:16
Linary
Linary requested changes Dec 24, 2019
View reviewed changes
src/test/java/com/baidu/hugegraph/unit/config/test-list-error.conf Outdated
@@ -0,0 +1 @@
group1.list_for_list_error=[1,2,3]
Copy link
Contributor

Choose a reason for hiding this comment

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

add an empty line

@javeme
tiny improve
a082d4b 
Change-Id: Ic40290aa9d0e9795aa27f5b25724ad3ed38c18f7
@javeme javeme mentioned this pull request Dec 29, 2019
Merged
@javeme
fix ConfigListOption values is empty
00f08c8 
Change-Id: Ifff6198d18488864ad65f33209a3187056675b7b
Linary
Linary previously approved these changes Dec 30, 2019
View reviewed changes
zhoney
zhoney requested changes Dec 30, 2019
View reviewed changes
src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java Outdated
}
}


Copy link
Contributor

Choose a reason for hiding this comment

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

delete one empty line

@javeme
delete unused empty line
585adb6 
Change-Id: Ic8e487e5f9f81b18d981d5d3b8b4c26664b35fec
@Linary Linary merged commit d78b728 into master Dec 31, 2019
@Linary Linary deleted the ConfigConvOption-bug branch December 31, 2019 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoney zhoney zhoney approved these changes

@Linary Linary Linary approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@javeme @Linary @zhoney