-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(config):resolve expected type in defaultResolver #1565 #2134
Conversation
case 'M': | ||
// We've already checked the map above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel a little confused about this, could you please describe why we need a case 'M'? and I can see the 'FNTfnt' matches the 'True|False|Null', so what does the 'OYoy' character match?
Thanks for your contribution, one suggestion, it's a good idea to add some description about this PR for reviewers to review :) fix #1565 |
Hey, @kagaya85 ,thanks for your suggestion. var resolveTable = make([]byte,256)
// Sign
resolveTable[int('+')] = 'S'
resolveTable[int('-')] = 'S'
// Digit
for _, c := range "0123456789" {
resolveTable[int(c)] = 'D'
}
// In map
for _, c := range "yYnNtTfFoO~" {
resolveTable[int(c)] = 'M'
}
// Float
resolveTable[int('.')] = '.' The resolveMap is a predefined map of potential types. "<<" is merge key,see: merge key. I'm sorry this part hasn't been completed yet, which makes you confused.And I remove "<<" temporarily. |
Codecov Report
@@ Coverage Diff @@
## main #2134 +/- ##
==========================================
- Coverage 79.08% 78.37% -0.71%
==========================================
Files 84 84
Lines 3696 3760 +64
==========================================
+ Hits 2923 2947 +24
- Misses 560 594 +34
- Partials 213 219 +6
Continue to review full report at Codecov.
|
Thanks for your explanation, and my question is, the One more thing, it looks like some additional test cases are needed to better explain and maintain the coverage. |
Description (what this PR does / why we need it):
Which issue(s) this PR fixes (resolves / be part of):
Other special notes for the reviewers: