code review 6742050: api: remove hard coded, add flag -a(alldecls) -e(allmet...

api: remove hard coded, add flag -a(alldecls) -e(allmethods). now -a flag work for GOROOT any package.

[visualfc, 2012-10-19 05:41:38]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go

[bradfitz, 2012-10-19 15:27:44]

Can this CL be cut up into pieces, sent one at a time? I'd prefer if the first version started with no new flags. Also, it needs new test files. I'm concerned that this is starting to duplicate too much code from the in-development go/types. My hope was to kill most of this code once Robert's type-checking code was completed. https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go File src/cmd/api/goapi.go (right): https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode48 src/cmd/api/goapi.go:48: alldecls = flag.Bool("a", false, "extract documentation for all package-level declarations") this would be allDecls. and "a" is too short. But I'm not sure what this is trying to say... extract "documentation"? https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode49 src/cmd/api/goapi.go:49: allmethods = flag.Bool("e", true, "show all embedded methods") allMethods. also, "e" is too short. I'd prefer to not add any more flags. https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode258 src/cmd/api/goapi.go:258: //check exported space after "//". // isExported returns whether the named symbol is exported. // If --include_unexported is set, all symbols are considered // exported. Which suggests maybe this should be named "includeSymbol" rather than isExported. https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode266 src/cmd/api/goapi.go:266: //check indent // isIdentifier returns whether ... But can't we use something in go/* packages for this? Robert (gri@) would know. https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode340 src/cmd/api/goapi.go:340: //func (w *Walker) hardCodedConstantType(name string) (typ string, ok bool) { no commented-out code https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode591 src/cmd/api/goapi.go:591: //check cgo C.xxx space after // https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode619 src/cmd/api/goapi.go:619: //check bool space after //. But this comment could be expanded with an example, because I don't understand that it's checking. https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode861 src/cmd/api/goapi.go:861: //if t, ok := w.hardCodedConstantType(ident.Name); ok { no commented-out code https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode... src/cmd/api/goapi.go:1120: //	if f.Recv != nil { no commented-out code https://codereview.appspot.com/6742050/diff/2002/src/cmd/api/goapi.go#newcode... src/cmd/api/goapi.go:1240: // merge to signale line single

[visualfc, 2012-10-22 13:36:49]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.

[gobot, 2012-10-22 21:41:39]

R=gri (assigned by bradfitz)

[visualfc, 2012-10-23 06:26:56]

Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[visualfc, 2012-10-23 08:52:20]

Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[visualfc, 2012-10-26 03:34:05]

Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[visualfc, 2012-10-26 03:35:17]

Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[visualfc, 2012-10-26 03:44:41]

Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[gri, 2012-10-26 05:07:30]

Let me know whew when you're ready for me to start looking at this. Thanks. - gri On Thu, Oct 25, 2012 at 8:44 PM, <VisualFC@gmail.com> wrote: > Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, > golang-dev@googlegroups.com), > > Please take another look. > > > http://codereview.appspot.com/**6742050/<http://codereview.appspot.com/6742050/> > 

[visualfc_gmail.com, 2012-10-26 13:10:17]

2012/10/26 Robert Griesemer <gri@golang.org>: > Let me know whew when you're ready for me to start looking at this. Thanks. > - gri > Now is work, all test pass. I'm clone to https://github.com/visualfc/goapi > > On Thu, Oct 25, 2012 at 8:44 PM, <VisualFC@gmail.com> wrote: >> >> Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, >> golang-dev@googlegroups.com), >> >> Please take another look. >> >> >> http://codereview.appspot.com/6742050/ > > 

[visualfc_gmail.com, 2012-10-27 02:00:12]

2012/10/26 visual fc <visualfc@gmail.com>: > 2012/10/26 Robert Griesemer <gri@golang.org>: >> Let me know whew when you're ready for me to start looking at this. Thanks. >> - gri >> Sorry cmd/api all codereview for next: goapi.go https://codereview.appspot.com/6742050/ goapi_test.go https://codereview.appspot.com/6785053/ testdata (p4.go) https://codereview.appspot.com/6752050/ Or clone https://github.com/visualfc/goapi > > Now is work, all test pass. > I'm clone to https://github.com/visualfc/goapi > >> >> On Thu, Oct 25, 2012 at 8:44 PM, <VisualFC@gmail.com> wrote: >>> >>> Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, >>> golang-dev@googlegroups.com), >>> >>> Please take another look. >>> >>> >>> http://codereview.appspot.com/6742050/ >> >> 

[gri, 2012-10-31 22:56:29]

Thanks. Sorry, this will have to wait just a tad longer. But i'll get to it. - gri On Fri, Oct 26, 2012 at 7:00 PM, visual fc <visualfc@gmail.com> wrote: > 2012/10/26 visual fc <visualfc@gmail.com>: >> 2012/10/26 Robert Griesemer <gri@golang.org>: >>> Let me know whew when you're ready for me to start looking at this. Thanks. >>> - gri >>> > > Sorry cmd/api all codereview for next: > goapi.go https://codereview.appspot.com/6742050/ > goapi_test.go https://codereview.appspot.com/6785053/ > testdata (p4.go) https://codereview.appspot.com/6752050/ > > Or clone https://github.com/visualfc/goapi > >> >> Now is work, all test pass. >> I'm clone to https://github.com/visualfc/goapi >> >>> >>> On Thu, Oct 25, 2012 at 8:44 PM, <VisualFC@gmail.com> wrote: >>>> >>>> Hello gri@golang.org (cc: bradfitz@golang.org, gobot@golang.org, >>>> golang-dev@googlegroups.com), >>>> >>>> Please take another look. >>>> >>>> >>>> http://codereview.appspot.com/6742050/ >>> >>> 

[bradfitz, 2012-11-08 07:58:17]

FYI I got bitten by goapi's type checker today, so I mailed out a quick simple fix in http://codereview.appspot.com/6818104 , but I still want to see this CL reviewed and submitted (or something), because I don't like goapi's current type checker.

[gri, 2012-11-08 16:47:43]

On my TODO list. Apologies for the delay. - gri On Wed, Nov 7, 2012 at 11:58 PM, <bradfitz@golang.org> wrote: > FYI I got bitten by goapi's type checker today, so I mailed out a quick > simple fix in http://codereview.appspot.com/6818104 , but I still want > to see this CL reviewed and submitted (or something), because I don't > like goapi's current type checker. > > https://codereview.appspot.com/6742050/ 

[visualfc, 2012-11-16 01:54:45]

Hello gri@golang.org, bradfitz@golang.org (cc: gobot@golang.org, golang-dev@googlegroups.com), Please take another look.

[bradfitz, 2013-06-04 22:40:55]

R=close This is too big of a change for me to maintain. I'd prefer people fork cmd/api elsewhere if they'd like to make big changes to it for their own use.