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.