http://codereview.appspot.com/6586074/diff/1/api/README File api/README (right): http://codereview.appspot.com/6586074/diff/1/api/README#newcode8 api/README:8: except.txt lists features that may disappear without breaking double ...
11 years, 6 months ago
(2012-10-04 00:48:33 UTC)
#3
LGTM on the code. Still unfortunate on going this route at all, but it's probably ...
11 years, 6 months ago
(2012-10-04 00:54:48 UTC)
#4
LGTM on the code.
Still unfortunate on going this route at all, but it's probably inevitable for
something else anyway, so we'll need the mechanism regardless.
https://codereview.appspot.com/6586074/diff/1/src/cmd/api/goapi.go
File src/cmd/api/goapi.go (right):
https://codereview.appspot.com/6586074/diff/1/src/cmd/api/goapi.go#newcode219
src/cmd/api/goapi.go:219: fmt.Fprintf(bw, "-%s\n", feature)
use different symbol failures vs permitted omissions?
I could see this being confusing if there's another minus failure and we can't
visually see which is the real failure.
Not a big deal, though.
On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike <r@golang.org> wrote: > I had ...
11 years, 6 months ago
(2012-10-04 01:00:41 UTC)
#6
On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike <r@golang.org> wrote:
> I had the same thought but couldn't think of a symbol. Suggest one and
> I'll use it.
U+1F4A9
On Wed, Oct 3, 2012 at 5:59 PM, Rob Pike <r@golang.org> wrote: > I had ...
11 years, 6 months ago
(2012-10-04 01:08:21 UTC)
#7
On Wed, Oct 3, 2012 at 5:59 PM, Rob Pike <r@golang.org> wrote:
> I had the same thought but couldn't think of a symbol. Suggest one and
> I'll use it.
>
~ ? Looks like a minus, and also looks kinda unhappy.
‽ On Thu, Oct 4, 2012 at 11:00 AM, David Symonds <dsymonds@golang.org> wrote: > On ...
11 years, 6 months ago
(2012-10-04 01:11:10 UTC)
#8
‽
On Thu, Oct 4, 2012 at 11:00 AM, David Symonds <dsymonds@golang.org> wrote:
> On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike <r@golang.org> wrote:
>
>> I had the same thought but couldn't think of a symbol. Suggest one and
>> I'll use it.
>
> U+1F4A9
As much as I loves me an interrobang, it doesn't really work here. It suggests ...
11 years, 6 months ago
(2012-10-04 01:15:44 UTC)
#9
As much as I loves me an interrobang, it doesn't really work here. It
suggests anger-and-confusion. We want to express misfortune/sadness.
On Wed, Oct 3, 2012 at 6:11 PM, Dave Cheney <dave@cheney.net> wrote:
> ‽
>
> On Thu, Oct 4, 2012 at 11:00 AM, David Symonds <dsymonds@golang.org>
> wrote:
> > On Thu, Oct 4, 2012 at 10:59 AM, Rob Pike <r@golang.org> wrote:
> >
> >> I had the same thought but couldn't think of a symbol. Suggest one and
> >> I'll use it.
> >
> > U+1F4A9
>
LGTM However: Please add an unexported method to package parse's type Node, so that future ...
11 years, 6 months ago
(2012-10-05 21:25:53 UTC)
#11
LGTM
However:
Please add an unexported method to package parse's type Node, so that
future extensions are guaranteed safe (because all the implementations
are in package parse) instead of just presumed safe.
Are we really sure about DotNode? It might be better to leave
DotNode's definition but mark it as unused and introduce a separate
DotNodePos with an embedded DotNode and Pos. What if someone is
rewriting the parse tree and creating a DotNode(false) somewhere?
On Sat, Oct 6, 2012 at 7:25 AM, Russ Cox <rsc@golang.org> wrote: > LGTM > ...
11 years, 6 months ago
(2012-10-05 21:42:06 UTC)
#12
On Sat, Oct 6, 2012 at 7:25 AM, Russ Cox <rsc@golang.org> wrote:
> LGTM
>
> However:
>
> Please add an unexported method to package parse's type Node, so that
> future extensions are guaranteed safe (because all the implementations
> are in package parse) instead of just presumed safe.
OK; CL in prep.
> Are we really sure about DotNode? It might be better to leave
> DotNode's definition but mark it as unused and introduce a separate
> DotNodePos with an embedded DotNode and Pos. What if someone is
> rewriting the parse tree and creating a DotNode(false) somewhere?
I'm comfortable. The package doc says, don't use this.
-rob
Issue 6586074: code review 6586074: cmd/api: add exception file
(Closed)
Created 11 years, 6 months ago by r
Modified 11 years, 6 months ago
Reviewers: rsc
Base URL:
Comments: 2