Issue 4642049: code review 4642049: goinstall: documentation for new remote repository behavior (Closed)
|
Created:
13 years, 9 months ago by adg Modified:
13 years, 8 months ago Reviewers:
CC:
rsc, quantumfyre, golang-dev Visibility:
Public. |
Descriptiongoinstall: 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/ #MessagesTotal messages: 13
Hello rsc, quantumfyre (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
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 also supports a few code hosting sites directly." perhaps? Or specially? Specifically? Doesn't seem to read quite right as is ... http://codereview.appspot.com/4642049/diff/1/src/cmd/goinstall/doc.go#newcode81 src/cmd/goinstall/doc.go:81: Packages at one of these sites must not include the VCS suffix. s/one/any/ ?
Sign in to reply to this message.
Hello rsc@golang.org, julian@quantumfyre.co.uk (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
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#newc... src/cmd/goinstall/doc.go:8: It maintains a list of public Go packages at http://godashboard.appspot.com/package. Do we need some extra words to explain that only "public" packages are reported to the dashboard? http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go#newc... src/cmd/goinstall/doc.go:76: Search for a Mercurial repository at "http://example.org/user/foo" or Should probably put http://example.org/user/foo.hg first, since that's what the code will actually try first.
Sign in to reply to this message.
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#newc... src/cmd/goinstall/doc.go:8: It maintains a list of public Go packages at http://godashboard.appspot.com/package. On 2011/06/30 11:50:00, quantumfyre wrote: > Do we need some extra words to explain that only "public" packages are reported > to the dashboard? Done. http://codereview.appspot.com/4642049/diff/6001/src/cmd/goinstall/doc.go#newc... src/cmd/goinstall/doc.go:76: Search for a Mercurial repository at "http://example.org/user/foo" or On 2011/06/30 11:50:00, quantumfyre wrote: > Should probably put http://example.org/user/foo.hg first, since that's what the > code will actually try first. Done, in that I reversed the order in the code.
Sign in to reply to this message.
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.g... src/cmd/goinstall/download.go:262: // vcsCheckout checks out repo into dst using vcs. s/vcsCheckout/checkoutRepo/
Sign in to reply to this message.
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.g... src/cmd/goinstall/download.go:262: // vcsCheckout checks out repo into dst using vcs. On 2011/07/01 11:32:54, quantumfyre wrote: > s/vcsCheckout/checkoutRepo/ Done.
Sign in to reply to this message.
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#newc... src/cmd/goinstall/doc.go:52: Goinstall does not use make. Makefiles are ignored by goinstall. Goinstall *does* use make. But it ignores makefiles. So let's change this to: Goinstall ignores Makefiles. 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 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 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://. 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 delete [moved up]
Sign in to reply to this message.
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", 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".
Sign in to reply to this message.
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): > > 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. > Goinstall *does* use make. But it ignores makefiles. > So let's change this to: > > Goinstall ignores Makefiles. > > 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 > 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://. There is currently no mention of https:// in download.go for "private" repositories ... or has this been added by another change? > > 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 > delete [moved up]
Sign in to reply to this message.
even after changeset: 8959:01c00d744ba3 user: Yasuhiro Matsumoto <mattn.jp@gmail.com> date: Fri Jul 01 10:11:33 2011 -0400 summary: cmd/goinstall: try to access via https. ?
Sign in to reply to this message.
*** 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.
|
