Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(302)

Issue 5577043: code review 5577043: go/doc: revert API change (per former discussion) and c... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 3 months ago by gri
Modified:
13 years, 3 months ago
Reviewers:
CC:
rsc, r, r2, golang-dev
Visibility:
Public.

Description

go/doc: revert API change (per former discussion) and cleanup Separating Method from Func made the code only more complicated without adding much to the useability/readability of the API. Reverted to where it was, but leaving the new method-specific fields Orig and Level. Former clients (godoc) of doc.Method only used the Func fields; and because Func was embedded, no changes are needed with respect to the removal of Method. Changed type of Func.Recv from ast.Expr to string. This was a long-standing TODO. Also implemented Func.Orig field (another TODO). No further go/doc API changes are expected for Go 1.

Patch Set 1 #

Patch Set 2 : diff -r 86434e8b59c5 https://code.google.com/p/go #

Patch Set 3 : diff -r 602b75a41065 https://code.google.com/p/go #

Patch Set 4 : diff -r 602b75a41065 https://code.google.com/p/go #

Total comments: 4

Patch Set 5 : diff -r 602b75a41065 https://code.google.com/p/go #

Patch Set 6 : diff -r 5a090946b7d2 https://code.google.com/p/go #

Patch Set 7 : diff -r 7de66de3961c https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -131 lines) Patch
M doc/go1.html View 1 2 3 4 5 1 chunk +2 lines, -3 lines 0 comments Download
M doc/go1.tmpl View 1 2 3 4 5 1 chunk +2 lines, -3 lines 0 comments Download
M lib/godoc/package.html View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/go/doc/doc.go View 1 2 3 4 4 chunks +13 lines, -17 lines 0 comments Download
M src/pkg/go/doc/filter.go View 1 2 chunks +1 line, -12 lines 0 comments Download
M src/pkg/go/doc/reader.go View 1 2 3 4 8 chunks +69 lines, -95 lines 0 comments Download

Messages

Total messages: 7
gri
Hello rsc@golang.org, r@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
13 years, 3 months ago (2012-01-25 21:29:08 UTC) #1
r2
go1.html should be on the list too (run make in the doc directory) and hg ...
13 years, 3 months ago (2012-01-25 21:30:46 UTC) #2
rsc
LGTM but wait for Rob for go1.html Hooray deleting code. http://codereview.appspot.com/5577043/diff/1003/src/pkg/go/doc/doc.go File src/pkg/go/doc/doc.go (right): http://codereview.appspot.com/5577043/diff/1003/src/pkg/go/doc/doc.go#newcode74 ...
13 years, 3 months ago (2012-01-25 21:33:30 UTC) #3
gri
PTAL http://codereview.appspot.com/5577043/diff/1003/src/pkg/go/doc/doc.go File src/pkg/go/doc/doc.go (right): http://codereview.appspot.com/5577043/diff/1003/src/pkg/go/doc/doc.go#newcode74 src/pkg/go/doc/doc.go:74: // New is destructive; the remaining AST is ...
13 years, 3 months ago (2012-01-25 21:42:46 UTC) #4
gri
PTAL. doc/go1.html added now. - gri ---------- Forwarded message ---------- From: <gri@golang.org> Date: Wed, Jan ...
13 years, 3 months ago (2012-01-25 22:11:10 UTC) #5
r
LGTM
13 years, 3 months ago (2012-01-26 00:33:42 UTC) #6
gri
13 years, 3 months ago (2012-01-26 00:48:11 UTC) #7
*** Submitted as 8eef2fc24233 ***

go/doc: revert API change (per former discussion) and cleanup

Separating Method from Func made the code only more complicated
without adding much to the useability/readability of the API.
Reverted to where it was, but leaving the new method-specific
fields Orig and Level.

Former clients (godoc) of doc.Method only used the Func fields;
and because Func was embedded, no changes are needed with respect
to the removal of Method.

Changed type of Func.Recv from ast.Expr to string. This was a
long-standing TODO. Also implemented Func.Orig field (another TODO).

No further go/doc API changes are expected for Go 1.

R=rsc, r, r
CC=golang-dev
http://codereview.appspot.com/5577043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b