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

Issue 12723043: code review 12723043: go/doc: Represent interface method declarations sam...

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by dtcaciuc
Modified:
9 years, 9 months ago
Visibility:
Public.

Description

go/doc: Represent interface method declarations same ways as regular struct methods. Updates issue 5860.

Patch Set 1 #

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

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

Total comments: 1

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -5 lines) Patch
M src/pkg/go/doc/reader.go View 1 2 3 3 chunks +51 lines, -4 lines 0 comments Download
M src/pkg/go/doc/testdata/error2.go View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/pkg/go/doc/testdata/error2.1.golden View 1 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 19
dtcaciuc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
10 years, 7 months ago (2013-08-11 05:51:26 UTC) #1
dtcaciuc
On 2013/08/11 05:51:26, dtcaciuc wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you to review ...
10 years, 7 months ago (2013-08-11 05:55:36 UTC) #2
dtcaciuc
On 2013/08/11 05:55:36, dtcaciuc wrote: > On 2013/08/11 05:51:26, dtcaciuc wrote: > > Hello mailto:golang-dev@googlegroups.com, ...
10 years, 7 months ago (2013-08-23 16:34:00 UTC) #3
bradfitz
No, just keeping pinging. A number of people are out on various holidays right now. ...
10 years, 7 months ago (2013-08-23 20:39:08 UTC) #4
rsc
not lgtm I apologize, but I think we should hold off on this until after ...
10 years, 6 months ago (2013-09-06 19:50:32 UTC) #5
dtcaciuc
On 2013/09/06 19:50:32, rsc wrote: > not lgtm > > I apologize, but I think ...
10 years, 6 months ago (2013-09-06 20:18:06 UTC) #6
rsc
Yes, you can just leave it open. After Go 1.2 is out please just reply ...
10 years, 6 months ago (2013-09-06 20:18:57 UTC) #7
Dominik Honnef
ping
10 years, 3 months ago (2013-12-10 22:08:59 UTC) #8
gobot
Replacing golang-dev with golang-codereviews.
10 years, 3 months ago (2013-12-20 16:26:00 UTC) #9
gobot
R=rsc@golang.org (assigned by bradfitz@golang.org)
10 years, 2 months ago (2014-01-14 23:27:11 UTC) #10
rsc
R=bgarcia@golang.org
10 years ago (2014-03-05 19:55:06 UTC) #11
rsc
R=close Please run 'hg mail' once Go 1.3 is out. Brad G, feel free to ...
9 years, 11 months ago (2014-04-17 02:39:58 UTC) #12
rsc
Hmm, apparently saying R=bgarcia@golang.org doesn't actually CC him. Maybe we should fix that.
9 years, 11 months ago (2014-04-17 02:40:49 UTC) #13
bgarcia
LGTM https://codereview.appspot.com/12723043/diff/6001/src/pkg/go/doc/reader.go File src/pkg/go/doc/reader.go (right): https://codereview.appspot.com/12723043/diff/6001/src/pkg/go/doc/reader.go#newcode612 src/pkg/go/doc/reader.go:612: // Returns instance and name of InterfaceType declaration ...
9 years, 11 months ago (2014-04-17 16:04:59 UTC) #14
rsc
Please don't submit until after 1.3.
9 years, 11 months ago (2014-04-17 16:12:15 UTC) #15
dtcaciuc
On 2014/04/17 16:04:59, bgarcia wrote: > LGTM > > https://codereview.appspot.com/12723043/diff/6001/src/pkg/go/doc/reader.go > File src/pkg/go/doc/reader.go (right): > ...
9 years, 11 months ago (2014-04-20 07:10:55 UTC) #16
Dominik Honnef
I noticed a little issue with this change. Now that methods from interfaces are listed ...
9 years, 9 months ago (2014-06-23 01:23:48 UTC) #17
dtcaciuc
On 2014/06/23 01:23:48, Dominik Honnef wrote: > I noticed a little issue with this change. ...
9 years, 9 months ago (2014-06-24 15:29:46 UTC) #18
Dominik Honnef
9 years, 9 months ago (2014-06-24 19:18:46 UTC) #19
On 2014/06/24 15:29:46, dtcaciuc wrote:
> On 2014/06/23 01:23:48, Dominik Honnef wrote:
> > I noticed a little issue with this change. Now that methods from interfaces
> are
> > listed as actual methods, they're no longer part of the code displayed for
the
> > type itself, so that it now looks like this:
> > 
> > > type Conn interface {
> > > }
> > 
> > which can be confusing at a first glance because it appears in a way similar
> to
> > what an empty interface would.
> 
> I see. It's probably important to have type declaration in the docs match the
> actual code.  If I undo the first part of the reader.go diff, I think methods
> would appear both inside the interface body _and_ be rendered as normal
methods.
> Would that work better?

Personally I think so, yes. But of course wait for input from people who
actually have a say in this.
Sign in to reply to this message.

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