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

Comments

Projects
None yet
6 participants
@bradfitz
Member

bradfitz commented Apr 11, 2016

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

This comment has been minimized.

Member

dsymonds commented Apr 11, 2016

Naming convention? Context keys are based on types.

@dsymonds dsymonds changed the title from contest: decide on naming convention for context key variables to context: decide on naming convention for context key variables Apr 11, 2016

@bradfitz

This comment has been minimized.

Member

bradfitz commented Apr 11, 2016

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

@dsymonds

This comment has been minimized.

Member

dsymonds commented Apr 11, 2016

I see.

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

@dsymonds

This comment has been minimized.

Member

dsymonds commented Apr 11, 2016

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

This comment has been minimized.

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

This comment has been minimized.

gopherbot commented Apr 11, 2016

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

gopherbot pushed a commit that referenced this issue Apr 11, 2016

net/http: add ServerContextKey to let a handler access its Server
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

This comment has been minimized.

gopherbot commented May 1, 2016

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

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

net/http: provide access to the listener address an HTTP request arri…
…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

This comment has been minimized.

Contributor

rsc commented May 18, 2016

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

@adg

This comment has been minimized.

Contributor

adg commented May 18, 2016

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

@bradfitz bradfitz closed this May 18, 2016

dmitshur added a commit to shurcooL/home that referenced this issue Aug 8, 2016

@Sajmani

This comment has been minimized.

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.