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

Issue 4642049: code review 4642049: goinstall: documentation for new remote repository behavior (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 9 months ago by adg
Modified:
13 years, 8 months ago
Reviewers:
CC:
rsc, quantumfyre, golang-dev
Visibility:
Public.

Description

goinstall: documentation for new remote repository behavior and tweaks

Patch Set 1 #

Total comments: 2

Patch Set 2 : diff -r 1cad1e8470ba https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r c78e6a668db0 https://go.googlecode.com/hg/ #

Total comments: 4

Patch Set 4 : diff -r c78e6a668db0 https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r c78e6a668db0 https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 6 : diff -r add294e751cb https://go.googlecode.com/hg/ #

Total comments: 8

Patch Set 7 : diff -r 13ec466f6670 https://go.googlecode.com/hg/ #

Patch Set 8 : diff -r 194ca0d677c7 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -78 lines) Patch
M src/cmd/goinstall/doc.go View 1 2 3 4 5 6 4 chunks +51 lines, -15 lines 0 comments Download
M src/cmd/goinstall/download.go View 1 2 3 4 5 6 7 chunks +87 lines, -63 lines 0 comments Download

Messages

Total messages: 13
adg
Hello rsc, quantumfyre (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 9 months ago (2011-06-21 10:47:22 UTC) #1
quantumfyre
http://codereview.appspot.com/4642049/diff/1/src/cmd/goinstall/doc.go File src/cmd/goinstall/doc.go (right): http://codereview.appspot.com/4642049/diff/1/src/cmd/goinstall/doc.go#newcode80 src/cmd/goinstall/doc.go:80: Goinstall also supports a few code hosting sites. "Goinstall ...
13 years, 9 months ago (2011-06-21 20:38:25 UTC) #2
adg
Hello rsc@golang.org, julian@quantumfyre.co.uk (cc: golang-dev@googlegroups.com), Please take another look.
13 years, 9 months ago (2011-06-22 02:02:40 UTC) #3
adg
PTAL
13 years, 8 months ago (2011-06-30 10:11:21 UTC) #4
quantumfyre
http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go File src/cmd/goinstall/doc.go (right): http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go#newcode8 src/cmd/goinstall/doc.go:8: It maintains a list of public Go packages at ...
13 years, 8 months ago (2011-06-30 11:50:00 UTC) #5
adg
http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go File src/cmd/goinstall/doc.go (right): http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go#newcode8 src/cmd/goinstall/doc.go:8: It maintains a list of public Go packages at ...
13 years, 8 months ago (2011-07-01 00:06:47 UTC) #6
quantumfyre
http://codereview.appspot.com/4642049/diff/11001/src/cmd/goinstall/download.go File src/cmd/goinstall/download.go (right): http://codereview.appspot.com/4642049/diff/11001/src/cmd/goinstall/download.go#newcode262 src/cmd/goinstall/download.go:262: // vcsCheckout checks out repo into dst using vcs. ...
13 years, 8 months ago (2011-07-01 11:32:54 UTC) #7
adg
http://codereview.appspot.com/4642049/diff/11001/src/cmd/goinstall/download.go File src/cmd/goinstall/download.go (right): http://codereview.appspot.com/4642049/diff/11001/src/cmd/goinstall/download.go#newcode262 src/cmd/goinstall/download.go:262: // vcsCheckout checks out repo into dst using vcs. ...
13 years, 8 months ago (2011-07-01 13:00:18 UTC) #8
rsc
LGTM http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go File src/cmd/goinstall/doc.go (right): http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go#newcode52 src/cmd/goinstall/doc.go:52: Goinstall does not use make. Makefiles are ignored ...
13 years, 8 months ago (2011-07-01 14:58:11 UTC) #9
quantumfyre
http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/download.go File src/cmd/goinstall/download.go (right): http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/download.go#newcode96 src/cmd/goinstall/download.go:96: check: "peek-remote", I don't know if this needs to ...
13 years, 8 months ago (2011-07-01 15:41:09 UTC) #10
quantumfyre
On 2011/07/01 14:58:11, rsc wrote: > LGTM > > http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go > File src/cmd/goinstall/doc.go (right): > ...
13 years, 8 months ago (2011-07-01 15:43:21 UTC) #11
rsc
even after changeset: 8959:01c00d744ba3 user: Yasuhiro Matsumoto <mattn.jp@gmail.com> date: Fri Jul 01 10:11:33 2011 -0400 ...
13 years, 8 months ago (2011-07-01 15:49:16 UTC) #12
adg
13 years, 8 months ago (2011-07-02 04:05:51 UTC) #13
*** Submitted as http://code.google.com/p/go/source/detail?r=338032227e73 ***

goinstall: documentation for new remote repository behavior and tweaks

R=rsc, julian
CC=golang-dev
http://codereview.appspot.com/4642049

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go
File src/cmd/goinstall/doc.go (right):

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go#newc...
src/cmd/goinstall/doc.go:52: Goinstall does not use make. Makefiles are ignored
by goinstall.
On 2011/07/01 14:58:11, rsc wrote:
> Goinstall *does* use make.  But it ignores makefiles.
> So let's change this to:
> 
> Goinstall ignores Makefiles.

Done.

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go#newc...
src/cmd/goinstall/doc.go:60: An import path may refer to an arbitrary source
repository by a suffix
On 2011/07/01 14:58:11, rsc wrote:
> Move all this new text down after the existing text.  The public sites are
> still the common case for goinstall.
> 
> 
> Goinstall recognizes packages from a few common code hosting sites:
> 
> [old text]
> 
> After a successful download and installation of one of these import paths,
> goinstall reports the installation to http://godashboard.appspot.com, which
> increments a count associated with the package and the time of its most
> recent installation. This mechanism powers the package list at
> http://godashboard.appspot.com/package, allowing Go programmers to learn about
> popular packages that might be worth looking at.	 
> The -dashboard=false flag disables this reporting.
> 
> For code hosted on other servers, goinstall recognizes the general form
> 
>     repository.vcs/path
> 
> as denoting the given repository, with or without the .vcs suffix, using
> the named version control system, and then the path inside that repository.
> The supported version control systems are:
> 
>     table
> 
> For example, 
> 
>     import "example.org/user/foo.hg"
> 
> denotes the root directory of the Mercurial repository at example.org/user/foo
> or foo.hg, and
> 
>     import "example.org/repo.git/foo/bar"
> 
> denotes the foo/bar directory of the Git repository at example.com/repo or
> repo.git.
> 
> When a version control system supports multiple protocols, goinstall tries
each
> in turn.
> For example, for Git it tries git://, then https://, then http://.

Done.

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/doc.go#newc...
src/cmd/goinstall/doc.go:131: After a successful download and installation of a
remote package from a
On 2011/07/01 14:58:11, rsc wrote:
> delete [moved up]

Done.

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/download.go
File src/cmd/goinstall/download.go (right):

http://codereview.appspot.com/4642049/diff/4003/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:96: check:             "peek-remote",
On 2011/07/01 15:41:09, quantumfyre wrote:
> I don't know if this needs to be done separately, but I just noticed that the
> git man page for peek-remote says:
> 
> "This command is deprecated; use git-ls-remote instead."
> 
> So, we should probably change this command to "ls-remote".

Done.
Sign in to reply to this message.

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