-
Notifications
You must be signed in to change notification settings - Fork 674
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
NOISSUE - Fix Readers logs #735
Conversation
Codecov Report
@@ Coverage Diff @@
## master #735 +/- ##
==========================================
- Coverage 85.39% 85.16% -0.23%
==========================================
Files 64 64
Lines 4230 4133 -97
==========================================
- Hits 3612 3520 -92
+ Misses 418 415 -3
+ Partials 200 198 -2
Continue to review full report at Codecov.
|
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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
readers/cassandra/messages_test.go
Outdated
result, err := reader.ReadAll(tc.chanID, tc.offset, tc.limit, tc.query) | ||
if err != nil { | ||
assert.Equal(t, nil, err, fmt.Sprintf("%s: expected no error got %s", desc, err.Error())) | ||
} |
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.
Don't use if err != nil {
. Use assert.Nil
instead of this if
and assert.Equal
.
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.
Thanks!
readers/influxdb/messages_test.go
Outdated
result, err := reader.ReadAll(tc.chanID, tc.offset, tc.limit, tc.query) | ||
if err != nil { | ||
assert.Equal(t, nil, err, fmt.Sprintf("%s: expected no error got %s", desc, err.Error())) | ||
} |
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.
Same here, use assert.Nil
instead.
readers/mongodb/messages_test.go
Outdated
result, err := reader.ReadAll(tc.chanID, tc.offset, tc.limit, tc.query) | ||
if err != nil { | ||
assert.Equal(t, nil, err, fmt.Sprintf("%s: expected no error got %s", desc, err.Error())) | ||
} |
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.
Same here, replace with assert.Nil
.
readers/cassandra/messages.go
Outdated
@@ -85,21 +87,21 @@ func (cr cassandraRepository) ReadAll(chanID string, offset, limit uint64, query | |||
} | |||
|
|||
if err := iter.Close(); err != nil { |
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.
Please ignore this error and move iter.Close()
to line 44 of this file. So it should be defer iter.Close()
.
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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!
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
* NOISSUE - Fix Readers logs Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
* NOISSUE - Fix Readers logs Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
* NOISSUE - Fix Readers logs Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com