Fix doc #1221
Fix doc #1221
Conversation
grpclb.go
Outdated
| @@ -96,7 +96,7 @@ const ( | |||
| GRPCLB | |||
| ) | |||
|
|
|||
| // AddrMetadataGRPCLB contains the information the name resolution for grpclb should provide. The | |||
| // AddrMetadataGRPCLB contains the information the name resolver for grpclb should provide. The | |||
| // name resolver used by grpclb balancer is required to provide this type of metadata in | |||
metadata/metadata.go
Outdated
| @@ -81,12 +81,12 @@ func Pairs(kv ...string) MD { | |||
| return md | |||
| } | |||
|
|
|||
| // Len returns the number of items in md. | |||
| // Len returns the number of items in MD. | |||
There was a problem hiding this comment.
Actually, "md" is correct here (that's the variable name; MD is the type name).
There was a problem hiding this comment.
The usage is inconsistent. In line 130 and several other places, MD is used. I'll change all of them to md then.
There was a problem hiding this comment.
Another question: do you think using the object name "md" by itself is sufficient or should more readable name be used?
There was a problem hiding this comment.
Godocs often refers to parameters or receivers by their name; I think this is fine.
metadata/metadata.go
Outdated
| @@ -51,7 +51,7 @@ func DecodeKeyValue(k, v string) (string, string, error) { | |||
| // two convenience functions New and Pairs to generate MD. | |||
| type MD map[string][]string | |||
|
|
|||
| // New creates a MD from given key-value map. | |||
| // New creates a md from given key-value map. | |||
There was a problem hiding this comment.
Here MD is the correct usage. It creates something of the type MD.
"md" should be used where it's referring to a parameter's name.
metadata/metadata.go
Outdated
| @@ -62,7 +62,7 @@ func New(m map[string]string) MD { | |||
| return md | |||
| } | |||
|
|
|||
| // Pairs returns an MD formed by the mapping of key, value ... | |||
| // Pairs returns an md formed by the mapping of key, value ... | |||
metadata/metadata.go
Outdated
| @@ -91,9 +91,9 @@ func (md MD) Copy() MD { | |||
| return Join(md) | |||
| } | |||
|
|
|||
| // Join joins any number of MDs into a single MD. | |||
| // Join joins any number of mds into a single md. | |||
There was a problem hiding this comment.
This was probably fine the way it was before. Or:
Join combines mds into a single MD.
metadata/metadata.go
Outdated
| // FromIncomingContext returns the incoming MD in ctx if it exists. The | ||
| // returned md should be immutable, writing to it may cause races. | ||
| // Modification should be made to the copies of the returned md. | ||
| // FromIncomingContext returns the incoming md in ctx if it exists. The |
There was a problem hiding this comment.
s/md/metadata/ here. It's referring to the concept and not a variable or type.
There was a problem hiding this comment.
Done. Also changed the two other mds to MDs.
metadata/metadata.go
Outdated
| func FromIncomingContext(ctx context.Context) (md MD, ok bool) { | ||
| md, ok = ctx.Value(mdIncomingKey{}).(MD) | ||
| return | ||
| } | ||
|
|
||
| // FromOutgoingContext returns the outgoing MD in ctx if it exists. The | ||
| // returned md should be immutable, writing to it may cause races. | ||
| // FromOutgoingContext returns the outgoing md in ctx if it exists. The |
metadata/metadata.go
Outdated
| // returned md should be immutable, writing to it may cause races. | ||
| // Modification should be made to the copies of the returned md. | ||
| // FromIncomingContext returns the incoming metadata in ctx if it exists. The | ||
| // returned MD should be immutable. Writing to it may cause races. |
There was a problem hiding this comment.
should be immutable -> should not be modified.
It is NOT immutable, and this might lead someone to believe that what is returned is a copy of the data and that modifying it is fine but doesn't change the underlying metadata (neither of which is true).
(Also in FromOutgoingContext.)
No description provided.