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

proposal: x/tools/cmd/godoc: hide deprecated things by default #17056

Open
bradfitz opened this Issue Sep 10, 2016 · 20 comments

Comments

Projects
None yet
10 participants
@bradfitz
Member

bradfitz commented Sep 10, 2016

Summary

https://golang.org/pkg/net/http/httputil/ has some garbage I'd like to hide. I'd like to hide everything about ClientConn and ServerConn

Background:

Before Go 1, we renamed the http package to net/http and in the process deleted some junk, and moved other junk to net/http/httputil, thinking that people might still need it. This was all pre-Go 1 when the APIs were changing daily, weekly, monthly. But we were trying to be nice anyway.

Turns out nobody needed that junk, though, even then, and not today:

https://github.com/search?utf8=%E2%9C%93&q=httputil.ClientConn&ref=simplesearch
https://github.com/search?utf8=%E2%9C%93&q=httputil.NewClientConn&type=Repositories&ref=searchresults
https://github.com/search?utf8=%E2%9C%93&q=httputil.ServerConn&type=Repositories&ref=searchresults
https://github.com/search?utf8=%E2%9C%93&q=httputil.NewServerConn&type=Repositories&ref=searchresults

Zero results.

I updated the docs some time ago:

https://golang.org/pkg/net/http/httputil/#ClientConn

... says simply:

ClientConn is an artifact of Go's early HTTP implementation. It is low-level, old, and unused by Go's current HTTP stack. We should have deleted it before Go 1.

Deprecated: Use Client or Transport in package net/http instead.

There are a couple good bits in there, though. ReverseProxy is very widely used, and the Dump* functions are often used in debugging.

Plan

Let godoc have configuration on public symbols to treat as if they were private. And then use that configuration for godoc.org to hide that junk, without deleting them from the package, so we don't violate the Go 1 compatibility promise. Think of this as a very strong form of deprecation.

If users really want to see it, they can use https://golang.org/pkg/net/http/httputil/?m=all to see all the junk.

FAQ

But Brad, this is a very slippery slope. What will we hide next?
Good question. There might be more to hide.

But why?
Because httputil looks embarrassing, and it looking like a trash heap prevents us from non-embarrassingly adding anything else to it. Let's clean up the house before we invite guests over.

/cc @griesemer @adg @broady @quentinmit @minux @campoy

@bradfitz bradfitz added the Proposal label Sep 10, 2016

@bradfitz bradfitz added this to the Proposal milestone Sep 10, 2016

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche Sep 10, 2016

Member

I'm all for cleaning up the documentation, but I don't like doing so by blacklist. What if godoc sorted items marked Deprecated below everything else and collapsed Deprecated items by default?

That would work with the (semi?) standard deprecation warnings for everyone without a special case for officially super-deprecated items. It wouldn't be totally hidden but it would be less messy, clearer that stuff is deprecated at a glance in general, and not too difficult for someone to look up deprecated docs if needed.

Something like:

func DumpRequest(req *http.Request, body bool) ([]byte, error)
func DumpRequestOut(req *http.Request, body bool) ([]byte, error)
func DumpResponse(resp *http.Response, body bool) ([]byte, error)
func NewChunkedReader(r io.Reader) io.Reader
func NewChunkedWriter(w io.Writer) io.WriteCloser
type BufferPool
type ReverseProxy
    func NewSingleHostReverseProxy(target *url.URL) *ReverseProxy
    func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request)

type ClientConn Deprecated (click to expand arrow icon thingy)
type ServerConn  Deprecated (click to expand arrow icon thingy)

where the {Client,Server}Conn's full documentation is at the bottom of the page, similarly collapsed.

Member

jimmyfrasche commented Sep 10, 2016

I'm all for cleaning up the documentation, but I don't like doing so by blacklist. What if godoc sorted items marked Deprecated below everything else and collapsed Deprecated items by default?

That would work with the (semi?) standard deprecation warnings for everyone without a special case for officially super-deprecated items. It wouldn't be totally hidden but it would be less messy, clearer that stuff is deprecated at a glance in general, and not too difficult for someone to look up deprecated docs if needed.

Something like:

func DumpRequest(req *http.Request, body bool) ([]byte, error)
func DumpRequestOut(req *http.Request, body bool) ([]byte, error)
func DumpResponse(resp *http.Response, body bool) ([]byte, error)
func NewChunkedReader(r io.Reader) io.Reader
func NewChunkedWriter(w io.Writer) io.WriteCloser
type BufferPool
type ReverseProxy
    func NewSingleHostReverseProxy(target *url.URL) *ReverseProxy
    func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request)

type ClientConn Deprecated (click to expand arrow icon thingy)
type ServerConn  Deprecated (click to expand arrow icon thingy)

where the {Client,Server}Conn's full documentation is at the bottom of the page, similarly collapsed.

@minux

This comment has been minimized.

Show comment
Hide comment
@minux

minux Sep 10, 2016

Member
Member

minux commented Sep 10, 2016

@bradfitz bradfitz changed the title from proposal: godoc: list of configurable symbols to hide by default to proposal: godoc: hide deprecated things by default Sep 10, 2016

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Sep 10, 2016

Member

@minux, SGTM. Hiding all deprecated things works for me with a mode to include them. Or have them simply be JavaScript-hidden is fine too, similar to Examples.

Member

bradfitz commented Sep 10, 2016

@minux, SGTM. Hiding all deprecated things works for me with a mode to include them. Or have them simply be JavaScript-hidden is fine too, similar to Examples.

@adg

This comment has been minimized.

Show comment
Hide comment
@adg

adg Sep 11, 2016

Contributor

SGTM

Contributor

adg commented Sep 11, 2016

SGTM

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 12, 2016

Good idea. Present the current docs by default, but still make legacy docs available to those who need them.

ghost commented Sep 12, 2016

Good idea. Present the current docs by default, but still make legacy docs available to those who need them.

@j7b

This comment has been minimized.

Show comment
Hide comment
@j7b

j7b Sep 12, 2016

👎 because the "zero results" is misleading, a quick search finds examples in the wild like https://github.com/samalba/skyproxy/blob/master/client/client.go and there's also cases where ClientConn is less painful than working around the net/http interfaces. Giving deprecated stuff a separate section is probably reasonable but hiding it is at the very least also misleading.

j7b commented Sep 12, 2016

👎 because the "zero results" is misleading, a quick search finds examples in the wild like https://github.com/samalba/skyproxy/blob/master/client/client.go and there's also cases where ClientConn is less painful than working around the net/http interfaces. Giving deprecated stuff a separate section is probably reasonable but hiding it is at the very least also misleading.

@broady

This comment has been minimized.

Show comment
Hide comment
@broady

broady Sep 12, 2016

Member

+1 for telling people that there are N things hidden. If they choose to "show all", let's also mark the deprecated stuff with an icon and group them together at the bottom.

What is the string that godoc will look for? Is it Deprecated: at the beginning of a comment line? I think I've also seen DEPRECATED used.

Member

broady commented Sep 12, 2016

+1 for telling people that there are N things hidden. If they choose to "show all", let's also mark the deprecated stuff with an icon and group them together at the bottom.

What is the string that godoc will look for? Is it Deprecated: at the beginning of a comment line? I think I've also seen DEPRECATED used.

@minux

This comment has been minimized.

Show comment
Hide comment
@minux

minux Sep 12, 2016

Member
Member

minux commented Sep 12, 2016

@dsnet

This comment has been minimized.

Show comment
Hide comment
@dsnet

dsnet Sep 12, 2016

Member

I have a minor concern with simply hiding deprecated items. If I'm reviewing some code that uses a deprecated feature that I'm unfamiliar with and then go to the godoc of the package, I'll be a little confused when cntrl-f doesn't show the deprecated feature in question. Remembering to show deprecated features is probably not my first thought. Is it possible for cntrl-f to search through (and show) javascript hidden elements?

However, I strongly believe that we should display deprecated features differently somehow in godoc.

Edit: Just a thought, maybe hide deprecated types, methods, and functions from the index? but still show them in package documentation? It's inconsistent, so I'm on the fence about this idea.

Member

dsnet commented Sep 12, 2016

I have a minor concern with simply hiding deprecated items. If I'm reviewing some code that uses a deprecated feature that I'm unfamiliar with and then go to the godoc of the package, I'll be a little confused when cntrl-f doesn't show the deprecated feature in question. Remembering to show deprecated features is probably not my first thought. Is it possible for cntrl-f to search through (and show) javascript hidden elements?

However, I strongly believe that we should display deprecated features differently somehow in godoc.

Edit: Just a thought, maybe hide deprecated types, methods, and functions from the index? but still show them in package documentation? It's inconsistent, so I'm on the fence about this idea.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Sep 12, 2016

Member

@dsnet, to address your Ctrl-F concern, we can make the JavaScript put at the bottom of the page:

Hiding 5 deprecated things: Foo, Bar, Type, Type.Method, Baz.

And each could be a link to #Foo, #Type.Method, etc, which like @minux said would open up the hidden element.

Member

bradfitz commented Sep 12, 2016

@dsnet, to address your Ctrl-F concern, we can make the JavaScript put at the bottom of the page:

Hiding 5 deprecated things: Foo, Bar, Type, Type.Method, Baz.

And each could be a link to #Foo, #Type.Method, etc, which like @minux said would open up the hidden element.

@dsnet

This comment has been minimized.

Show comment
Hide comment
@dsnet

dsnet Sep 12, 2016

Member

@broady

What is the string that godoc will look for? Is it Deprecated: at the beginning of a comment line? I think I've also seen DEPRECATED used.

It seems that #10909 settled on Deprecated: as the convention. There's even some discussion on that issue about altering godoc to handle that string specially, but obviously we didn't really move forward with that work (till now).

Member

dsnet commented Sep 12, 2016

@broady

What is the string that godoc will look for? Is it Deprecated: at the beginning of a comment line? I think I've also seen DEPRECATED used.

It seems that #10909 settled on Deprecated: as the convention. There's even some discussion on that issue about altering godoc to handle that string specially, but obviously we didn't really move forward with that work (till now).

@adg

This comment has been minimized.

Show comment
Hide comment
@adg

adg Oct 24, 2016

Contributor

The plan:

  • Deprecated items are not listed in the index
  • A separate short list of deprecated symbols is presented after the index
  • Deprecated symbols are documented in a separate section, below the other code
  • The deprecated symbol docs are collapsed by default, as with examples
  • A direct link to a deprecated symbol shows the docs already expanded, as with examples

The go doc command also needs to address this, consistently with godoc. That's a separate issue.

Contributor

adg commented Oct 24, 2016

The plan:

  • Deprecated items are not listed in the index
  • A separate short list of deprecated symbols is presented after the index
  • Deprecated symbols are documented in a separate section, below the other code
  • The deprecated symbol docs are collapsed by default, as with examples
  • A direct link to a deprecated symbol shows the docs already expanded, as with examples

The go doc command also needs to address this, consistently with godoc. That's a separate issue.

@dsnet

This comment has been minimized.

Show comment
Hide comment
@dsnet

dsnet Oct 25, 2016

Member

Just wondering. How do you plan on handling deprecated fields in structs (e.g., zip.FileHeader.CompressedSize)?

Member

dsnet commented Oct 25, 2016

Just wondering. How do you plan on handling deprecated fields in structs (e.g., zip.FileHeader.CompressedSize)?

@adg

This comment has been minimized.

Show comment
Hide comment
@adg

adg Oct 25, 2016

Contributor

We'll probably punt on it. I don't see an elegant way of dealing with it.

On 26 October 2016 at 07:36, Joe Tsai notifications@github.com wrote:

Just wondering. How do you plan on handling deprecated fields in structs
(e.g., zip.FileHeader.CompressedSize
https://golang.org/pkg/archive/zip/#FileHeader)?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17056 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIDilZ8x6KzPBEvawTEs4A63fliYPeDsks5q3mhkgaJpZM4J51O6
.

Contributor

adg commented Oct 25, 2016

We'll probably punt on it. I don't see an elegant way of dealing with it.

On 26 October 2016 at 07:36, Joe Tsai notifications@github.com wrote:

Just wondering. How do you plan on handling deprecated fields in structs
(e.g., zip.FileHeader.CompressedSize
https://golang.org/pkg/archive/zip/#FileHeader)?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17056 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIDilZ8x6KzPBEvawTEs4A63fliYPeDsks5q3mhkgaJpZM4J51O6
.

@mibk

This comment has been minimized.

Show comment
Hide comment
@mibk

mibk Oct 26, 2016

Contributor

Maybe just use strike out text on the struct fields? Apologies if this was already discussed or declined.

Contributor

mibk commented Oct 26, 2016

Maybe just use strike out text on the struct fields? Apologies if this was already discussed or declined.

@rakyll

This comment has been minimized.

Show comment
Hide comment
@rakyll

rakyll Dec 9, 2016

Member

+1 to strike through. There are already similar issues filed against godoc.org and golint by the way.

golang/lint#238
golang/gddo#456

Member

rakyll commented Dec 9, 2016

+1 to strike through. There are already similar issues filed against godoc.org and golint by the way.

golang/lint#238
golang/gddo#456

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Dec 9, 2016

Member

Strike-through seems too aggressive or at least too unreadable. I'd do a JS expando before I did strike-through.

Member

bradfitz commented Dec 9, 2016

Strike-through seems too aggressive or at least too unreadable. I'd do a JS expando before I did strike-through.

@mibk

This comment has been minimized.

Show comment
Hide comment
@mibk

mibk Dec 9, 2016

Contributor

As @adg pointed out, there seems to be no elegant way of handling deprecated fields in structs. I totally agree with using "JS expando" for all other elements. Strike-through is just a possible solution for struct fields.

Contributor

mibk commented Dec 9, 2016

As @adg pointed out, there seems to be no elegant way of handling deprecated fields in structs. I totally agree with using "JS expando" for all other elements. Strike-through is just a possible solution for struct fields.

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot Oct 11, 2017

Change https://golang.org/cl/70110 mentions this issue: encoding/json: use Deprecated markers

gopherbot commented Oct 11, 2017

Change https://golang.org/cl/70110 mentions this issue: encoding/json: use Deprecated markers

gopherbot pushed a commit that referenced this issue Oct 11, 2017

encoding/json: use Deprecated markers
In #10909, it was decided that "Deprecated:" is a magic string for
tools (e.g., #17056 for godoc) to detect deprecated identifiers.
Use those convention instead of custom written prose.

Change-Id: Ia514fc3c88fc502e86c6e3de361c435f4cb80b22
Reviewed-on: https://go-review.googlesource.com/70110
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
@dsnet

This comment has been minimized.

Show comment
Hide comment
@dsnet

dsnet Oct 16, 2017

Member

As part of my work on #18342, I'm close to a working prototype of this. I'm going to self-assign this

Member

dsnet commented Oct 16, 2017

As part of my work on #18342, I'm close to a working prototype of this. I'm going to self-assign this

@dsnet dsnet self-assigned this Oct 16, 2017

@agnivade agnivade changed the title from proposal: godoc: hide deprecated things by default to proposal: x/tools/cmd/godoc: hide deprecated things by default May 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment