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.

Sign up for GitHub

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

Jump to bottom

x/pkgsite: hide deprecated things from documentation page by default #40850

Closed
julieqiu opened this issue Aug 17, 2020 · 33 comments
Closed

x/pkgsite: hide deprecated things from documentation page by default #40850

julieqiu opened this issue Aug 17, 2020 · 33 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite
Milestone
pkgsite/unplanned

Comments

@julieqiu
Copy link
Member

Related to #17056, we should hide deprecated things from the pkg.go.dev doc page by default.
@julieqiu julieqiu added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite labels Aug 17, 2020
@julieqiu julieqiu mentioned this issue Aug 17, 2020
Closed
@gopherbot gopherbot added this to the Unreleased milestone Aug 17, 2020
@nasirhm
Copy link

nasirhm commented Aug 18, 2020 •
edited
Loading

I would like to work on it.

cc: @julieqiu

@julieqiu
Copy link
Member Author

Thanks @nasirhm! If you're interested in working on this feature, please share a plan on this issue first. It looks like @dsnet might have also worked on a prototype in the past based on #17056 (comment).

@myitcv
Copy link
Member

myitcv commented Mar 29, 2021

@julieqiu @dsnet - is there any update on this?

@myitcv
Copy link
Member

myitcv commented Apr 20, 2021

Relaying a message from 0xjnml on Slack:

That's IMO a very bad idea. Please imagine the confusion when someone wants to check the docs for old code using something deprecated. It makes the UX frustrating, non-deterministic and unreliable. The docs shown on the web page should be the same as what is in the sources, only slightly modified by applying styles as usual. An added list of deprecated things is okay

@seebs
Copy link
Contributor

seebs commented Apr 20, 2021

That's a good point, it seems really bad to have documentation for a thing just magically go missing. Clear deprecation warnings would be good.

@myitcv
Copy link
Member

myitcv commented Apr 20, 2021

The counterpoint to my mind is that for new users it is probably the better thing to hide deprecated identifiers by default, so as not to clutter the API/documentation.

@fgm
Copy link

fgm commented Apr 21, 2021

It also means that pkg.go.dev and godoc should probably use more explicit styling on symbols marked with a canonical Deprecated: comment, maybe like graying out the symbols and adding an explicit list of deprecations like the index.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312270 mentions this issue: internal/godoc: don't render doc on worker

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 21, 2021
@jba
internal/godoc: don't render doc on worker
3b55c14 
The Package.Render method is only used on the worker to get the
synopsis and other doc-related information. Ever since we started
rendering doc on the frontend, the worker has ignored the rendered
documentation.

So stop rendering the doc, and rename the method to DocInfo to reflect
that.

One minor consequence is that we no longer flag packages with
excessively large doc as having a bad status. As far as the worker is
concerned, they are fine; the error will manifest on the frontend (and
we will serve a "documentation too large" page). This is all good: we
only used a distinct status in this case so we could reprocess if our
limits changed, but these modules no longer need reprocessing.

For golang/go#40850

Change-Id: I3c7c49f0beb7a6d8a37daabdce75f83ef108eddb
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312270
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312329 mentions this issue: internal/doc: rename RenderParts to Render

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312430 mentions this issue: internal/godoc: renaem RenderPartsFromUnit

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 22, 2021
@jba
internal/doc: rename RenderParts to Render
762b7a6 
For golang/go#40850

Change-Id: I8cb2de424d8fc6b1ee4f48fa7ed68048340942e2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312329
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 22, 2021
@jba
internal/godoc: rename RenderPartsFromUnit
802d4e6 
For golang/go#40850

Change-Id: I82b7b3cd683eedacc6e49e66bca5b5a2f5a19ae8
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312430
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312649 mentions this issue: internal/godoc/dochmtl: remove unused Render things

@jba
Copy link
Contributor

jba commented Apr 22, 2021 •
edited
Loading

Here is the design as presented in #17056 (comment):

  • 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

I'll check off items here as they're done.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312691 mentions this issue: internal/godoc/dochtml: remove deprecated symbols from index

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312693 mentions this issue: internal/godoc/dochtml: remove deprecated symbols from mobile outline

@dsnet
Copy link
Member

dsnet commented Apr 22, 2021 •
edited
Loading

@jba. Awesome to see you working on this! What type of declarations can be hidden?

  • the entire package?
  • a type?
  • a standalone constant/variable?
  • a constant/variable that is part of larger const/var block?
  • a method on a type?
  • a function?
  • a field in a struct type?
  • a method in an interface type?

@dsnet
Copy link
Member

dsnet commented Apr 22, 2021

Assuming functions are hidden, if I navigate to https://pkg.go.dev/github.com/golang/protobuf/ptypes#Duration, will it auto-unhide the Duration documentation?

@jba
Copy link
Contributor

jba commented Apr 22, 2021

@dsnet, not the entire package, not a field, not a method in an interface type. Top level decls. (Should work for those in a group, will check.)

Yes, hidden things should expand if navigated to. We are following adg's plan as repeated above.

@dsnet
Copy link
Member

dsnet commented Apr 22, 2021

I agree that entire packages should not be hidden. I think we should explore what a UI might look like that hides struct fields and interface methods, but that come can be after the initial first step of making it work with top-level decls.

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 22, 2021
@jba
internal/godoc/dochmtl: remove unused Render things
2ccf768 
- Remove old Render method
- Remove unit.tmpl
- Re-organize template loading
- Rename RenderParts to Render

For golang/go#40850

Change-Id: Ie27a54b3553ef100b39d472f52b400f8d8d8392e
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312649
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 22, 2021
@jba
internal/godoc/dochtml: remove deprecated symbols from index
e0ffc7d 
If a symbol is deprecated, don't display it in the outline.

This CL only handles the non-mobile outline.

For golang/go#40850

Change-Id: Ic758ffa5d811b2a2287c550dddadc38d0a322937
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/312691
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@jba
Copy link
Contributor

jba commented Apr 22, 2021

@dsnet, I see you've been here before: indeed, I cannot easily separate a const/var in a block. You'd have to deprecate the entire block. Not great, but the alternative would do too much violence to our fork of go/doc. Better to wait for go/doc/v2.

@dsnet
Copy link
Member

dsnet commented Apr 22, 2021

@jba, Yea, I last looked at this 3 years ago, but I recall const/var blocks being a challenge. I suspect a good UI for hiding entries in a const/var block could be suitable suitable for hiding struct fields and interface methods, though I don't know exactly how that would look.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313032 mentions this issue: content/static/html/doc/body.tmpl: move decl doc to sub-template

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313034 mentions this issue: content/static/html/doc/body.tmpl: add deprecated symbols

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313030 mentions this issue: internal/godoc/dochtml: compare with golden

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313031 mentions this issue: internal/godoc/dochtml: split out deprecated symbols

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313033 mentions this issue: content/static/html/doc/body.tmpl: add suffix, remove ids

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 23, 2021
@jba
internal/godoc/dochtml: compare with golden
750994b 
Add a comparison with a golden file to the test, for
more thorough coverage.

Delete tests that are obsoleted by having a golden.

Factor out golden comparison to a separate package.

For golang/go#40850

Change-Id: Iac7b4c0e1c4aadad690d8d5118a6861408614aa2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/313030
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 23, 2021
@jba
internal/godoc/dochtml: split out deprecated symbols
2a10a51 
After parsing the doc, remove deprecated symbols and store them
separately.

This will keep them out of the index and let us display them
separately in the body.

For golang/go#40850

Change-Id: Ia80ef20ef79ddad853e31bad4f4fc9805fd43b2c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/313031
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 23, 2021
@jba
content/static/html/doc/body.tmpl: move decl doc to sub-template
e0d7aa7 
Make a separate template for the actual docs of consts, vars, funcs
and types.

This makes way for rendering deprecated decls separately.

For golang/go#40850

Change-Id: I4f5331b897325c35b67d195d168f7d3fc93a8a8c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/313032
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314089 mentions this issue: internal/godoc/dochtml: remove deprecated split

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314129 mentions this issue: internal/godoc/dochtml,content/static: preliminary deprecated display

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 27, 2021
@jba
internal/godoc/dochtml: remove deprecated split
bd1d5f7 
Remove the code that split out deprecated symbols.

For golang/go#40850

Change-Id: Ibf3cc5bf57a0e450853f9130c489e3f1d78900b6
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/314089
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 27, 2021
@jba
internal/godoc/dochtml,content/static: preliminary deprecated display
462deaf 
For golang/go#40850

Change-Id: If1631c6d56c694dbeee28e4475b0ddaec316d990
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/314129
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314592 mentions this issue: content/static: show deprecated symbols specially

gopherbot pushed a commit to golang/pkgsite that referenced this issue Apr 28, 2021
@jba
content/static: show deprecated symbols specially
15d2d7e 
For golang/go#40850

Change-Id: I1b7d2ac03dbba61646a63edc373fe2b9db993756
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/314592
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323969 mentions this issue: content/static: deprecated symbols ui updates

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 1, 2021
@jamalc
content/static: deprecated symbols ui updates
b5f5aef 
Deprecated symbol sections are updated to match
UX design. Deprecated sections will expand when
navigated to from the index or a direct link.

For golang/go#40850

Change-Id: I2108a28d8a4c1655fcad414a239f46605a725673
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/323969
Trust: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@jamalc
Copy link

jamalc commented Jun 14, 2021

Here is the UI for deprecated symbols.

In the index --
Screen Shot 2021-06-14 at 9 33 14 AM

Collapsed by default in the doc --
Screen Shot 2021-06-14 at 9 35 19 AM

Expanded in the doc --
Screen Shot 2021-06-14 at 9 35 35 AM

@jamalc
Copy link

jamalc commented Jun 15, 2021

This change is now live on https://beta.pkg.go.dev/.

@julieqiu
Copy link
Member Author

This change is live on https://pkg.go.dev.

@myitcv
Copy link
Member

myitcv commented Sep 13, 2021

Such a great change, thank you to everyone who worked on this!

@golang golang locked and limited conversation to collaborators Sep 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite
Projects
None yet
Milestone
pkgsite/unplanned
Development

No branches or pull requests
9 participants
@fgm @seebs @myitcv @julieqiu @dsnet @gopherbot @jamalc @jba @nasirhm