-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
.*: replace grpc.Dial
with grpc.NewClient
in test files
#7609
.*: replace grpc.Dial
with grpc.NewClient
in test files
#7609
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7609 +/- ##
==========================================
- Coverage 81.73% 81.70% -0.03%
==========================================
Files 361 361
Lines 27817 27817
==========================================
- Hits 22735 22729 -6
- Misses 3871 3877 +6
Partials 1211 1211 |
grpc.Dial
with grpc.NewClient
in test files
@hanut19 please complete your CLA Authorization. Also, please ensure following the convention for PR description and title for next PRs |
if err != nil { | ||
t.Fatalf("failed to dial local test server: %v", err) | ||
t.Fatalf("failed to NewClient() local test server: %v", err) |
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.
failed to create new client for local test server
. Please apply this everywhere
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.
done please check
fa6d247
to
2344f90
Compare
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
@dfawley for second review |
Note that the CLAs are currently broken, so we can't merge this. |
if err != nil { | ||
log.Fatalf("Failed to dial: %v", err) | ||
log.Fatalf("Failed to NewClient: %v", err) |
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.
"dial" is a verb. "NewClient" is not. Please fix the errors that don't have proper grammar.
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.
@hanut19 failed to create new client
. Similar to what you changed elsewhere. I think you missed this one
another PR is raised #7640 |
Partially address: #7049
Update tests where only direct replacement is enough to replace
grpc.Dial
withgrpc.NewClient
in test filesRELEASE NOTES: None