-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
[MRG+1] Fix missing values #3819
Conversation
Could you please include an automated test covering your changes? |
Codecov Report
@@ Coverage Diff @@
## master #3819 +/- ##
==========================================
+ Coverage 85.42% 85.43% +<.01%
==========================================
Files 169 169
Lines 9635 9637 +2
Branches 1433 1434 +1
==========================================
+ Hits 8231 8233 +2
Misses 1156 1156
Partials 248 248
|
Codecov Report
@@ Coverage Diff @@
## master #3819 +/- ##
==========================================
- Coverage 85.42% 84.89% -0.54%
==========================================
Files 169 169
Lines 9635 9637 +2
Branches 1433 1434 +1
==========================================
- Hits 8231 8181 -50
- Misses 1156 1202 +46
- Partials 248 254 +6
|
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.
Looks good to me. Failing tests are all unrelated.
tests/test_loader.py
Outdated
@@ -709,28 +746,28 @@ def test_nested_load_item(self): | |||
|
|||
|
|||
class SelectJmesTestCase(unittest.TestCase): | |||
test_list_equals = { |
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 would avoid these indentation fixes, being unrelated to the actual changes.
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.
Fixed, thanks :)
As discussed with @Gallaecio, bugs are random and not caused by this pull request code changes. |
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.
LGTM
Issue: #3804