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

More golint cleanups #47

Merged
merged 2 commits into from
Apr 18, 2018
Merged

More golint cleanups #47

merged 2 commits into from
Apr 18, 2018

Conversation

knweiss
Copy link
Contributor

@knweiss knweiss commented Apr 18, 2018

Please review.

This makes to code at little more idiomatic and fixes:

server_metrics.go:140:6: func streamRpcType should be streamRPCType
server_test.go:97:6: range var testId should be testID
This fixes:
metric_options.go:7:6: exported type CounterOption should have comment or be unexported
metric_options.go:18:1: exported function WithConstLabels should have comment or be unexported
metric_options.go:24:6: exported type HistogramOption should have comment or be unexported
metric_options.go:31:1: exported function WithHistogramConstLabels should have comment or be unexported
@knweiss
Copy link
Contributor Author

knweiss commented Apr 18, 2018

BTW: golint also complains about this:

util.go:16:2: exported const Unary should have comment (or a comment on this block) or be unexported

This is the code:

type grpcType string

const (
        Unary        grpcType = "unary"
        ClientStream grpcType = "client_stream"
        ServerStream grpcType = "server_stream"
        BidiStream   grpcType = "bidi_stream"
)

Is it intentional that these consts are exported (the type grpcType is not)?
Shall we add a godoc string or unexport them?

@codecov-io
Copy link

codecov-io commented Apr 18, 2018

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #47   +/-   ##
=======================================
  Coverage   78.59%   78.59%           
=======================================
  Files           8        8           
  Lines         271      271           
=======================================
  Hits          213      213           
  Misses         49       49           
  Partials        9        9
Impacted Files Coverage Δ
metric_options.go 16.66% <ø> (ø) ⬆️
server_metrics.go 80.19% <100%> (ø) ⬆️

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 3a0e610...c03e2ac. Read the comment docs.

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

brancz commented Apr 18, 2018

lgtm

@brancz brancz merged commit a63527e into grpc-ecosystem:master Apr 18, 2018
@brancz
Copy link
Collaborator

brancz commented Apr 18, 2018

changing whether they are exported would be a breaking change, which we can't do in v1.2

generally though it would be good to not export anything unnecessary 🙂

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.

None yet

3 participants