Skip to content
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

context: decide on naming convention for context key variables #15229

Closed
bradfitz opened this issue Apr 11, 2016 · 10 comments
Closed

context: decide on naming convention for context key variables #15229

bradfitz opened this issue Apr 11, 2016 · 10 comments
Milestone

Comments

@bradfitz
Copy link
Contributor

Before Go 1.7, let's be sure we're happy with the naming convention for context key values.

/cc @adg @Sajmani @dsymonds

@bradfitz bradfitz self-assigned this Apr 11, 2016
@bradfitz bradfitz added this to the Go1.7 milestone Apr 11, 2016
@dsymonds
Copy link
Contributor

Naming convention? Context keys are based on types.

@dsymonds dsymonds changed the title contest: decide on naming convention for context key variables context: decide on naming convention for context key variables Apr 11, 2016
@bradfitz
Copy link
Contributor Author

The naming convention for well-known context key values we use. See https://go-review.googlesource.com/#/c/21829 as an example.

@dsymonds
Copy link
Contributor

I see.

I'll suggest using <name describing value>Key.

@dsymonds
Copy link
Contributor

Though we've often found it nicer to expose WithFoo and Foo functions that wrap context.WithValue and ctx.Value calls instead of exposing the context key directly.

@adg
Copy link
Contributor

adg commented Apr 11, 2016

Are you talking about ContextKeyServer?

I'd prefer ServerContextKey. But what are other potential key names in this context? (eg FooContextKey)

@gopherbot
Copy link

CL https://golang.org/cl/21829 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 11, 2016
Fixes #12438
Updates #15229 (to decide context key variable naming convention)

Change-Id: I3ba423e91b689e232143247d044495a12c97a7d2
Reviewed-on: https://go-review.googlesource.com/21829
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/22672 mentions this issue.

gopherbot pushed a commit that referenced this issue May 1, 2016
…ved on

This adds a context key named LocalAddrContextKey (for now, see #15229) to
let users access the net.Addr of the net.Listener that accepted the connection
that sent an HTTP request. This is similar to ServerContextKey which provides
access to the *Server. (A Server may have multiple Listeners)

Fixes #6732

Change-Id: I74296307b68aaaab8df7ad4a143e11b5227b5e62
Reviewed-on: https://go-review.googlesource.com/22672
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc
Copy link
Contributor

rsc commented May 18, 2016

@adg, @Sajmani, @bradfitz, are you happy now?

@adg
Copy link
Contributor

adg commented May 18, 2016

Brad seems to have gone with FooContextKey which is a-ok with me.

dmitshur added a commit to shurcooL/home that referenced this issue Aug 8, 2016
@Sajmani
Copy link
Contributor

Sajmani commented Aug 29, 2016

For exported context keys, FooKey or FooContextKey seem fine to me. But my personal preference is to encapsulate the key and provide foo.NewContext(ctx, val)/foo.FromContext(ctx) or pkg.ContextWithFoo(ctx, val)/pkg.FooFromContext(ctx).

@golang golang locked and limited conversation to collaborators Aug 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants