code review 4592059: misc/godashboard: Allow github.com/foo/bar/baz for repo...

misc/godashboard: Accept sub-directories for goinstall's report.
Fixed issue 1155.

[mattn, 2011-06-15 07:11:17]

Hello golang-dev@googlecode.com, adg@golang.org (cc:
golang-dev@googlegroups.com),

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

[rsc, 2011-06-15 12:53:28]

I don't believe this is the right fix.  This only affects
what the dashboard accepts, not what goinstall reports.

[adg, 2011-06-15 23:00:54]

Agree with Russ. There's a more complete fix here and it involves both
goinstall and the dashboard.

On 15 June 2011 17:11,  <mattn.jp@gmail.com> wrote:
> Reviewers: golang-dev_googlecode.com, adg,
>
> Message:
> Hello golang-dev@googlecode.com, adg@golang.org (cc:
> golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> http://go.googlecode.com/hg/
>
>
> Description:
> misc/godashboard: Allow github.com/foo/bar/baz for reporting.
> Fixed issue 1957.
>
> Please review this at http://codereview.appspot.com/4592059/
>
> Affected files:
>  M misc/dashboard/godashboard/package.py
>
>
> Index: misc/dashboard/godashboard/package.py
> ===================================================================
> --- a/misc/dashboard/godashboard/package.py
> +++ b/misc/dashboard/godashboard/package.py
> @@ -45,7 +45,7 @@
>
>  re_bitbucket =
> re.compile(r'^bitbucket\.org/[a-z0-9A-Z_.\-]+/[a-z0-9A-Z_.\-]+$')
>  re_googlecode = re.compile(r'^[a-z0-9\-]+\.googlecode\.com/(svn|hg)$')
> -re_github = re.compile(r'^github\.com/[a-z0-9A-Z_.\-]+/[a-z0-9A-Z_.\-]+$')
> +re_github =
> re.compile(r'^github\.com/[a-z0-9A-Z_.\-]+(/[a-z0-9A-Z_.\-]+)+$')
>  re_launchpad =
>
re.compile(r'^launchpad\.net/([a-z0-9A-Z_.\-]+(/[a-z0-9A-Z_.\-]+)?|~[a-z0-9A-Z_.\-]+/(\+junk|[a-z0-9A-Z_.\-]+)/[a-z0-9A-Z_.\-]+)(/[a-z0-9A-Z_.\-/]+)?$')
>
>
> @@ -54,10 +54,9 @@
>         check_url = 'http://' + path + '/?cmd=heads'
>         web = 'http://' + path + '/'
>     elif re_github.match(path):
> -        # github doesn't let you fetch the .git directory anymore.
> -        # fetch .git/info/refs instead, like git clone would.
> -        check_url = 'http://'+path+'.git/info/refs'
> -        web = 'http://' + path
> +        m = re_github_web.match(path)
> +        check_url = 'https://raw.github.com/' + m.group(1) + '/' +
> m.group(2) + '/master/'
> +        web = 'http://github.com/' + m.group(1) + '/' + m.group(2)
>     elif re_googlecode.match(path):
>         check_url = 'http://'+path
>         web = 'http://code.google.com/p/' + path[:path.index('.')]
>
>
>

[mattn, 2011-06-16 04:19:06]

On 2011/06/15 23:00:54, adg wrote:
> Agree with Russ. There's a more complete fix here and it involves both
> goinstall and the dashboard.
> 
> On 15 June 2011 17:11,  <mailto:mattn.jp@gmail.com> wrote:
> > Reviewers: http://golang-dev_googlecode.com, adg,
> >
> > Message:
> > Hello mailto:golang-dev@googlecode.com, mailto:adg@golang.org (cc:
> > mailto:golang-dev@googlegroups.com),
> >
> > I'd like you to review this change to
> > http://go.googlecode.com/hg/
> >
> >
> > Description:
> > misc/godashboard: Allow github.com/foo/bar/baz for reporting.
> > Fixed issue 1957.
> >
> > Please review this at http://codereview.appspot.com/4592059/
> >
> > Affected files:
> > &nbsp;M misc/dashboard/godashboard/package.py
> >
> >
> > Index: misc/dashboard/godashboard/package.py
> > ===================================================================
> > --- a/misc/dashboard/godashboard/package.py
> > +++ b/misc/dashboard/godashboard/package.py
> > @@ -45,7 +45,7 @@
> >
> > &nbsp;re_bitbucket =
> > re.compile(r'^bitbucket\.org/[a-z0-9A-Z_.\-]+/[a-z0-9A-Z_.\-]+$')
> > &nbsp;re_googlecode = re.compile(r'^[a-z0-9\-]+\.googlecode\.com/(svn|hg)$')
> > -re_github = re.compile(r'^github\.com/[a-z0-9A-Z_.\-]+/[a-z0-9A-Z_.\-]+$')
> > +re_github =
> > re.compile(r'^github\.com/[a-z0-9A-Z_.\-]+(/[a-z0-9A-Z_.\-]+)+$')
> > &nbsp;re_launchpad =
> >
>
re.compile(r'^launchpad\.net/([a-z0-9A-Z_.\-]+(/[a-z0-9A-Z_.\-]+)?|~[a-z0-9A-Z_.\-]+/(\+junk|[a-z0-9A-Z_.\-]+)/[a-z0-9A-Z_.\-]+)(/[a-z0-9A-Z_.\-/]+)?$')
> >
> >
> > @@ -54,10 +54,9 @@
> > &nbsp; &nbsp; &nbsp; &nbsp; check_url = 'http://' + path + '/?cmd=heads'
> > &nbsp; &nbsp; &nbsp; &nbsp; web = 'http://' + path + '/'
> > &nbsp; &nbsp; elif re_github.match(path):
> > - &nbsp; &nbsp; &nbsp; &nbsp;# github doesn't let you fetch the .git
directory anymore.
> > - &nbsp; &nbsp; &nbsp; &nbsp;# fetch .git/info/refs instead, like git clone
would.
> > - &nbsp; &nbsp; &nbsp; &nbsp;check_url = 'http://'+path+'.git/info/refs'
> > - &nbsp; &nbsp; &nbsp; &nbsp;web = 'http://' + path
> > + &nbsp; &nbsp; &nbsp; &nbsp;m = re_github_web.match(path)
> > + &nbsp; &nbsp; &nbsp; &nbsp;check_url = 'https://raw.github.com/' +
m.group(1) + '/' +
> > m.group(2) + '/master/'
> > + &nbsp; &nbsp; &nbsp; &nbsp;web = 'http://github.com/' + m.group(1) + '/' +
m.group(2)
> > &nbsp; &nbsp; elif re_googlecode.match(path):
> > &nbsp; &nbsp; &nbsp; &nbsp; check_url = 'http://'+path
> > &nbsp; &nbsp; &nbsp; &nbsp; web = 'http://code.google.com/p/' +
path[:path.index('.')]
> >
> >
> >

added goinstall/download.go

[adg, 2011-06-16 05:44:50]

Looks better

http://codereview.appspot.com/4592059/diff/9001/misc/dashboard/godashboard/pa...
File misc/dashboard/godashboard/package.py (right):

http://codereview.appspot.com/4592059/diff/9001/misc/dashboard/godashboard/pa...
misc/dashboard/godashboard/package.py:58: check_url = 'https://raw.github.com/'
+ m.group(1) + '/' + m.group(2) + '/master/'
What's raw.github.com?

[mattn, 2011-06-16 06:42:29]

http://codereview.appspot.com/4592059/diff/9001/misc/dashboard/godashboard/pa...
File misc/dashboard/godashboard/package.py (right):

http://codereview.appspot.com/4592059/diff/9001/misc/dashboard/godashboard/pa...
misc/dashboard/godashboard/package.py:58: check_url = 'https://raw.github.com/'
+ m.group(1) + '/' + m.group(2) + '/master/'
On 2011/06/16 05:44:50, adg wrote:
> What's raw.github.com?

raw.github.com provide raw files in the repo.

[adg, 2011-06-20 01:07:37]

LGTM

But I'll just make the dashboard change, as the change to download.go will be
taken care of in 4650043

[mattn, 2011-06-20 01:40:42]

On 2011/06/20 01:07:37, adg wrote:
> LGTM
> 
> But I'll just make the dashboard change, as the change to download.go will be
> taken care of in 4650043

I stripped off download.go from this CL.

[adg, 2011-06-20 01:45:11]

On 2011/06/20 01:40:42, mattn wrote:
> On 2011/06/20 01:07:37, adg wrote:
> > LGTM
> > 
> > But I'll just make the dashboard change, as the change to download.go will
be
> > taken care of in 4650043
> 
> I stripped off download.go from this CL.

Have you done an "hg upload" since?

[mattn, 2011-06-20 01:52:07]

On 2011/06/20 01:45:11, adg wrote:
> On 2011/06/20 01:40:42, mattn wrote:
> > On 2011/06/20 01:07:37, adg wrote:
> > > LGTM
> > > 
> > > But I'll just make the dashboard change, as the change to download.go will
> be
> > > taken care of in 4650043
> > 
> > I stripped off download.go from this CL.
> 
> Have you done an "hg upload" since?

Yes, done. I changed CL description.

[adg, 2011-06-20 04:46:40]

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

misc/godashboard: Accept sub-directories for goinstall's report.
Fixed issue 1155.

R=golang-dev, adg, rsc
CC=golang-dev
http://codereview.appspot.com/4592059

Committer: Andrew Gerrand <adg@golang.org>