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

x/net/websocket: Naming of TLSConfig #16447

Open
evrimoztamur opened this issue Jul 20, 2016 · 6 comments

Comments

@evrimoztamur
Copy link

commented Jul 20, 2016

Server class of net/httpConfig uses the name TLSConfig for its tls.config variable whereas Config class of x/net/websocket uses TlsConfig. Can the inconsistent naming be resolved, or will it be kept due to backwards compatibility?

@evrimoztamur evrimoztamur changed the title Notation of TLSConfig x/net/websocket: Naming of TLSConfig Jul 20, 2016

@minux

This comment has been minimized.

Copy link
Member

commented Jul 20, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 21, 2016

@campoy, can you query BigQuery and see how much code would break if we fixed the naming?

@bradfitz bradfitz added this to the Unreleased milestone Jul 21, 2016

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2016

I found 685 repos that could be broken by this change

SELECT COUNT(DISTINCT repo_name)
FROM
(
  SELECT id, content
  FROM [campoy-github:go_files.contents]
  WHERE content CONTAINS '"golang.org/x/net/websocket"'
  AND content CONTAINS 'TlsConfig'
  AND NOT content CONTAINS 'Copyright 2009 The Go Authors'
) as content JOIN 
(
  SELECT id, repo_name, path
  FROM [campoy-github:go_files.files]
  WHERE NOT path CONTAINS 'golang.org/x/net/websocket'

) as files
ON content.id = files.id

I can provide with a list of the files that actually use the field by its name if you want, but I checked a couple of them and they would indeed break.

Example from this file:

        port := 80
        if protocol == "wss" {
            port = 443
        }
        wsConfig.Location.Host = fmt.Sprintf("%s:%d", hostName, port)
        wsConfig.TlsConfig = tlsConfig

        ws, err := websocket.DialConfig(wsConfig)
        if err != nil {
            return "", err
        }
@evrimoztamur

This comment has been minimized.

Copy link
Author

commented Jul 23, 2016

What about internal references?

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2016

I'm dropping all files in both the package and any vendored copy of it.

@evrimoztamur

This comment has been minimized.

Copy link
Author

commented Dec 18, 2016

Requesting an update on this. Will this change be committed regardless of the compatibility issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.