Navigation Menu

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
Open

x/net/websocket: Naming of TLSConfig #16447

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

Comments

@evrimoztamur
Copy link

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
Copy link
Member

minux commented Jul 20, 2016 via email

@bradfitz
Copy link
Contributor

@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
Copy link
Contributor

campoy 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
Copy link
Author

What about internal references?

@campoy
Copy link
Contributor

campoy commented Jul 23, 2016

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

@evrimoztamur
Copy link
Author

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants