code review 5572062: dashboard: add sub-repositories to init list

dashboard: add sub-repositories to init list
dashboard: add Kind to Package struct
dashboard: add kind parameter to /packages handler

[adg, 2012-01-25 21:28:59]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg

[rsc, 2012-01-25 21:40:15]

http://codereview.appspot.com/5572062/diff/1/misc/dashboard/app/build/init.go
File misc/dashboard/app/build/init.go (right):

http://codereview.appspot.com/5572062/diff/1/misc/dashboard/app/build/init.go...
misc/dashboard/app/build/init.go:25: "codereview/cmd/hgpatch",
This code should be written in terms of repos, since that is 
the unit of download.  If this just listed the subrepos 
(codereview, crypto, net), then the loop can do, for each:

hg clone http://code.google.com/p/go.xxx
$GOROOT/src/pkg/code.google.com/p/go.xxx

and then after downloading them all (because there
might be dependencies):

go get -u code.google.com/p/go.xxx/...  # will sync to right tag
go test code.google.com/p/go.xxx/...

I don't think it's tenable to add to this list every time we make
a new package.  It is tempting to take shortcuts here, but when
are we going to fix this if we do?

[bsiegert, 2012-01-26 07:50:15]

On Wed, Jan 25, 2012 at 22:40,  <rsc@golang.org> wrote:
>
> go get -u code.google.com/p/go.xxx/...  # will sync to right tag
> go test code.google.com/p/go.xxx/...
>
> I don't think it's tenable to add to this list every time we make
> a new package.  It is tempting to take shortcuts here, but when
> are we going to fix this if we do?

How about adding globbing to the go tool? Something like
go test 'code.google.com/p/go.xxx/*'

--Benny.


-- 
The first essential in chemistry is that you should perform practical
work and conduct experiments, for he who performs not practical work
nor makes experiments will never attain the least degree of mastery.
        -- Abu Musa Jabir ibn Hayyan (721-815)

[rog, 2012-01-26 09:12:40]

On 26 January 2012 07:50, Benny Siegert <bsiegert@gmail.com> wrote:
> On Wed, Jan 25, 2012 at 22:40,  <rsc@golang.org> wrote:
>>
>> go get -u code.google.com/p/go.xxx/...  # will sync to right tag
>> go test code.google.com/p/go.xxx/...
>>
>> I don't think it's tenable to add to this list every time we make
>> a new package.  It is tempting to take shortcuts here, but when
>> are we going to fix this if we do?
>
> How about adding globbing to the go tool? Something like
> go test 'code.google.com/p/go.xxx/*'

the go tool has globbing already (that's what the "..." signifies),
and very useful it is too.

interesting point though - i don't think it's possible to use
that globbing to match all packages starting with "code.google.com/p/go."
as there's no way of quoting the first dot.

[adg, 2012-01-26 21:25:56]

On 26 January 2012 20:12, roger peppe <rogpeppe@gmail.com> wrote:
> On 26 January 2012 07:50, Benny Siegert <bsiegert@gmail.com> wrote:
>> On Wed, Jan 25, 2012 at 22:40,  <rsc@golang.org> wrote:
>>>
>>> go get -u code.google.com/p/go.xxx/...  # will sync to right tag
>>> go test code.google.com/p/go.xxx/...
>>>
>>> I don't think it's tenable to add to this list every time we make
>>> a new package.  It is tempting to take shortcuts here, but when
>>> are we going to fix this if we do?
>>
>> How about adding globbing to the go tool? Something like
>> go test 'code.google.com/p/go.xxx/*'
>
> the go tool has globbing already (that's what the "..." signifies),
> and very useful it is too.
>
> interesting point though - i don't think it's possible to use
> that globbing to match all packages starting with "code.google.com/p/go."
> as there's no way of quoting the first dot.

... only works as a path element. It's not like *.

[rsc, 2012-01-26 21:26:24]

On Thu, Jan 26, 2012 at 16:25, Andrew Gerrand <adg@golang.org> wrote:
> ... only works as a path element. It's not like *.

is is exactly like *, except that it matches / too

[adg, 2012-01-26 23:26:06]

PTAL

Corresponding gobuilder change coming shortly.

https://codereview.appspot.com/5572062/diff/1/misc/dashboard/app/build/init.go
File misc/dashboard/app/build/init.go (right):

https://codereview.appspot.com/5572062/diff/1/misc/dashboard/app/build/init.g...
misc/dashboard/app/build/init.go:25: "codereview/cmd/hgpatch",
On 2012/01/25 21:40:15, rsc wrote:
> This code should be written in terms of repos, since that is 
> the unit of download.  If this just listed the subrepos 
> (codereview, crypto, net), then the loop can do, for each:
> 
> hg clone http://code.google.com/p/go.xxx
> $GOROOT/src/pkg/code.google.com/p/go.xxx
> 
> and then after downloading them all (because there
> might be dependencies):
> 
> go get -u code.google.com/p/go.xxx/...  # will sync to right tag
> go test code.google.com/p/go.xxx/...
> 
> I don't think it's tenable to add to this list every time we make
> a new package.  It is tempting to take shortcuts here, but when
> are we going to fix this if we do?
> 

Done.

[rog, 2012-01-27 10:29:44]

On 26 January 2012 21:26, Russ Cox <rsc@golang.org> wrote:
> On Thu, Jan 26, 2012 at 16:25, Andrew Gerrand <adg@golang.org> wrote:
>> ... only works as a path element. It's not like *.
>
> is is exactly like *, except that it matches / too

'*' can be quoted with a backslash if you need to.

maybe if "...." was interpreted as "." followed by "..." rather than
"..." followed by ".", then four-dots would be more useful
(package paths rarely have a dot at the end).

[rsc, 2012-01-27 22:50:15]

LGTM

http://codereview.appspot.com/5572062/diff/4002/misc/dashboard/app/build/init.go
File misc/dashboard/app/build/init.go (right):

http://codereview.appspot.com/5572062/diff/4002/misc/dashboard/app/build/init...
misc/dashboard/app/build/init.go:24: "crypto",
Please add "image" to this list, and it can just fail until image exists.

[adg, 2012-01-30 00:59:12]

*** Submitted as http://code.google.com/p/go/source/detail?r=ddf9499f63c6 ***

dashboard: add sub-repositories to init list
dashboard: add Kind to Package struct
dashboard: add kind parameter to /packages handler

R=rsc, bsiegert, rogpeppe
CC=golang-dev
http://codereview.appspot.com/5572062