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

Issue 125580044: code review 125580044: cmd/go: add go generate (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by r
Modified:
9 years, 6 months ago
Reviewers:
brainman, rsc, roktas
CC:
rsc, brainman, golang-codereviews
Visibility:
Public.

Description

cmd/go: add go generate First cut. Works well enough to support yacc via http://codereview.appspot.com/125620044.

Patch Set 1 #

Total comments: 40

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

Total comments: 11

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

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

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

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

Total comments: 4

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

Total comments: 5

Patch Set 8 : diff -r fca097989c4f439dc4197fc924c23bf3494e1fb6 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+491 lines, -2 lines) Patch
M src/cmd/go/doc.go View 1 2 3 4 5 6 7 3 chunks +91 lines, -2 lines 0 comments Download
A src/cmd/go/generate.go View 1 2 3 4 5 6 7 1 chunk +340 lines, -0 lines 0 comments Download
M src/cmd/go/main.go View 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/go/test.bash View 1 chunk +27 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/generate/test1.go View 1 chunk +13 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/generate/test2.go View 1 chunk +10 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/generate/test3.go View 1 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 16
r
Hello rsc (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 7 months ago (2014-08-20 21:28:45 UTC) #1
rsc
https://codereview.appspot.com/125580044/diff/1/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/1/src/cmd/go/generate.go#newcode25 src/cmd/go/generate.go:25: addBuildFlags(cmdGenerate) move this into the func init below https://codereview.appspot.com/125580044/diff/1/src/cmd/go/generate.go#newcode30 ...
9 years, 7 months ago (2014-08-23 22:16:31 UTC) #2
r
please answer question about source order (see TODO in new source) https://codereview.appspot.com/125580044/diff/1/src/cmd/go/generate.go File src/cmd/go/generate.go (right): ...
9 years, 7 months ago (2014-08-23 23:24:10 UTC) #3
r
Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
9 years, 7 months ago (2014-08-23 23:25:10 UTC) #4
brainman
https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go#newcode328 src/cmd/go/generate.go:328: cmd.Env = append(cmd.Env, fmt.Sprintf("GODIR=%s", g.dir)) What if your current ...
9 years, 7 months ago (2014-08-24 01:26:06 UTC) #5
rsc
https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go#newcode51 src/cmd/go/generate.go:51: $GODIR Should we remove $GODIR entirely? Generators can call ...
9 years, 7 months ago (2014-08-24 02:52:35 UTC) #6
r
https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/20001/src/cmd/go/generate.go#newcode51 src/cmd/go/generate.go:51: $GODIR On 2014/08/24 02:52:35, rsc wrote: > Should we ...
9 years, 7 months ago (2014-08-24 06:33:38 UTC) #7
r
Hello rsc@golang.org, alex.brainman@gmail.com (cc: golang-codereviews@googlegroups.com), Please take another look.
9 years, 7 months ago (2014-08-24 06:33:51 UTC) #8
brainman
LGTM Alex https://codereview.appspot.com/125580044/diff/100001/src/cmd/go/doc.go File src/cmd/go/doc.go (right): https://codereview.appspot.com/125580044/diff/100001/src/cmd/go/doc.go#newcode253 src/cmd/go/doc.go:253: $GODIR You have removed $GODIR from cmdGenerate. ...
9 years, 7 months ago (2014-08-24 06:50:09 UTC) #9
r
https://codereview.appspot.com/125580044/diff/100001/src/cmd/go/doc.go File src/cmd/go/doc.go (right): https://codereview.appspot.com/125580044/diff/100001/src/cmd/go/doc.go#newcode253 src/cmd/go/doc.go:253: $GODIR On 2014/08/24 06:50:09, brainman wrote: > You have ...
9 years, 6 months ago (2014-08-24 14:01:54 UTC) #10
r
Hello rsc@golang.org, alex.brainman@gmail.com (cc: golang-codereviews@googlegroups.com), Please take another look.
9 years, 6 months ago (2014-08-24 14:02:34 UTC) #11
rsc
LGTM https://codereview.appspot.com/125580044/diff/120001/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/120001/src/cmd/go/generate.go#newcode90 src/cmd/go/generate.go:90: If any generator returns an error exit status, ...
9 years, 6 months ago (2014-08-24 17:43:00 UTC) #12
rsc
go1.4.txt too
9 years, 6 months ago (2014-08-24 17:43:08 UTC) #13
r
https://codereview.appspot.com/125580044/diff/120001/src/cmd/go/generate.go File src/cmd/go/generate.go (right): https://codereview.appspot.com/125580044/diff/120001/src/cmd/go/generate.go#newcode90 src/cmd/go/generate.go:90: If any generator returns an error exit status, "go ...
9 years, 6 months ago (2014-08-24 18:34:12 UTC) #14
r
*** Submitted as https://code.google.com/p/go/source/detail?r=901a1adaf852 *** cmd/go: add go generate First cut. Works well enough to ...
9 years, 6 months ago (2014-08-24 18:34:17 UTC) #15
roktas
9 years, 6 months ago (2014-08-25 19:17:57 UTC) #16
Message was sent while issue was closed.
On 2014/08/24 18:34:17, r wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=901a1adaf852 ***
> 
> cmd/go: add go generate
> First cut.
> 
> Works well enough to support yacc via
>         http://codereview.appspot.com/125620044.
> 
> LGTM=alex.brainman, rsc
> R=rsc, alex.brainman
> CC=golang-codereviews
> https://codereview.appspot.com/125580044

FYI, minor typo at
https://code.google.com/p/go/source/browse/src/cmd/go/generate.go#164

s/encouter/encounter/
Sign in to reply to this message.

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