Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Some cleanups for megacheck warnings #45

Merged
merged 3 commits into from
Apr 18, 2018

Conversation

knweiss
Copy link
Contributor

@knweiss knweiss commented Apr 17, 2018

Please review (esp the grpc.Code() commit).

See the individual commit messages for details.

I've only tested with go test so far.

knweiss added 3 commits April 17, 2018 23:20
This fixes:
server_test.go:300:14:warning: grpc.Errorf is deprecated: use status.Errorf instead.  (SA1019) (megacheck)
server_test.go:305:10:warning: grpc.Errorf is deprecated: use status.Errorf instead.  (SA1019) (megacheck)
This fixes:
util.go:40:5: should omit comparison to bool constant, can be simplified to !mInfo.IsClientStream (S1002)
util.go:40:38: should omit comparison to bool constant, can be simplified to !mInfo.IsServerStream (S1002)
util.go:43:5: should omit comparison to bool constant, can be simplified to mInfo.IsClientStream (S1002)
util.go:43:37: should omit comparison to bool constant, can be simplified to !mInfo.IsServerStream (S1002)
util.go:46:5: should omit comparison to bool constant, can be simplified to !mInfo.IsClientStream (S1002)
util.go:46:38: should omit comparison to bool constant, can be simplified to mInfo.IsServerStream (S1002)
This fixes:
client_metrics.go:115:19: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
client_metrics.go:126:20: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
client_metrics.go:163:21: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
client_test.go:164:49: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
client_test.go:192:49: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
server_metrics.go:107:19: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
server_metrics.go:120:19: grpc.Code is deprecated: use status.FromError and Code method instead.  (SA1019)
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@knweiss
Copy link
Contributor Author

knweiss commented Apr 17, 2018

Please see #40 regarding the Travis-CI failure.

@knweiss knweiss mentioned this pull request Apr 17, 2018
@brancz
Copy link
Collaborator

brancz commented Apr 18, 2018

@knweiss you also need to sign the CLA for us to be able to merge these changes. Also even if flaky I'd prefer to see the tests pass.

@brancz brancz mentioned this pull request Apr 18, 2018
@knweiss
Copy link
Contributor Author

knweiss commented Apr 18, 2018

@brancz done

@googlebot
Copy link

CLAs look good, thanks!

@codecov-io
Copy link

Codecov Report

Merging #45 into master will increase coverage by 0.04%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
+ Coverage    77.4%   77.45%   +0.04%     
==========================================
  Files           8        8              
  Lines         270      275       +5     
==========================================
+ Hits          209      213       +4     
- Misses         52       53       +1     
  Partials        9        9
Impacted Files Coverage Δ
server_metrics.go 77.14% <100%> (+0.44%) ⬆️
client_metrics.go 74.19% <66.66%> (-0.26%) ⬇️
util.go 69.23% <66.66%> (ø) ⬆️

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 67e2f48...2d55642. Read the comment docs.

@brancz
Copy link
Collaborator

brancz commented Apr 18, 2018

lgtm

@brancz brancz merged commit 5f56f5a into grpc-ecosystem:master Apr 18, 2018
@@ -37,13 +37,13 @@ func splitMethodName(fullMethodName string) (string, string) {
}

func typeFromMethodInfo(mInfo *grpc.MethodInfo) grpcType {
if mInfo.IsClientStream == false && mInfo.IsServerStream == false {
if !mInfo.IsClientStream && !mInfo.IsServerStream {
Copy link
Collaborator

Choose a reason for hiding this comment

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

lol (:

Copy link
Collaborator

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

nice

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants