Issue 48730043: code review 48730043: doc/install.html: get rid of explicit version number in... (Closed)
Descriptiondoc/install.html: get rid of explicit version number in docs.
So that we don't need to update the file each time a new version is released.
Patch Set 1 #Patch Set 2 : diff -r 0dee786a73f7 https://code.google.com/p/go #Patch Set 3 : diff -r 0dee786a73f7 https://code.google.com/p/go #Patch Set 4 : diff -r 0dee786a73f7 https://code.google.com/p/go #
Total comments: 1
Patch Set 5 : diff -r d067c6de4ec3 https://code.google.com/p/go #
Total comments: 2
Patch Set 6 : diff -r 6d3bdbd27761 https://code.google.com/p/go #Patch Set 7 : diff -r 6d3bdbd27761 https://code.google.com/p/go #Patch Set 8 : diff -r 6d3bdbd27761 https://code.google.com/p/go #Patch Set 9 : diff -r 6d3bdbd27761 https://code.google.com/p/go #MessagesTotal messages: 19
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
Please use VERSION not VER. LGTM assuming <i> is valid inside a <pre> block. > On 8 Jan 2014, at 9:49, minux.ma@gmail.com wrote: > > Reviewers: golang-codereviews, > > Message: > Hello golang-codereviews@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > doc/install.html: get rid of explicit version number in docs. > So that we don't need to update the file each time a new version is > released. > > Please review this at https://codereview.appspot.com/48730043/ > > Affected files (+1, -1 lines): > M doc/install.html > > > Index: doc/install.html > =================================================================== > --- a/doc/install.html > +++ b/doc/install.html > @@ -76,7 +76,7 @@ > </p> > > <pre> > -tar -C /usr/local -xzf go1.1.linux-amd64.tar.gz > +tar -C /usr/local -xzf go<i>VER</i>.<i>OS</i>-<i>ARCH</i>.tar.gz > </pre> > > <p> > > > -- > You received this message because you are subscribed to the Google Groups "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out.
Sign in to reply to this message.
On 2014/01/07 22:54:10, dfc wrote: > Please use VERSION not VER. Done. > LGTM assuming <i> is valid inside a <pre> block. Yes, I've verified that it is working. However I'm not sure if <i> is the best way to emphasize this. I'm open to suggestions.
Sign in to reply to this message.
How about <b>$VERSION</b> > On 8 Jan 2014, at 10:07, minux.ma@gmail.com wrote: > >> On 2014/01/07 22:54:10, dfc wrote: >> Please use VERSION not VER. > Done. >> LGTM assuming <i> is valid inside a <pre> block. > Yes, I've verified that it is working. However I'm not sure if <i> is > the best way to emphasize this. I'm open to suggestions. > > https://codereview.appspot.com/48730043/
Sign in to reply to this message.
On 2014/01/07 23:09:15, dfc wrote: > How about <b>$VERSION</b> I think if we are using $var, it doesn't need to emphasize anymore. so now i settled down on just: tar -C /usr/local -xzf go$VERSION.$OS-$ARCH.tar.gz
Sign in to reply to this message.
In retrospect, putting a hypen between go and $VERSION would have read better, but such is life. If possible I think it's worth using <em> or <b> or something to encourage readers to no just blindly copy the string into their command line. On second thought it was a mistake to suggest $VERSION, not VERSION. The former, if copied onto the command line will expand to go.-.tar.gz, but the latter would be goVERSION.OS-ARCH.tar.gz which would give a cleaner not found error. On Wed, Jan 8, 2014 at 10:34 AM, <minux.ma@gmail.com> wrote: > On 2014/01/07 23:09:15, dfc wrote: > >> How about <b>$VERSION</b> >> > I think if we are using $var, it doesn't need to emphasize anymore. > so now i settled down on just: > tar -C /usr/local -xzf go$VERSION.$OS-$ARCH.tar.gz > > https://codereview.appspot.com/48730043/ >
Sign in to reply to this message.
On Tue, Jan 7, 2014 at 6:39 PM, Dave Cheney <dave@cheney.net> wrote: > In retrospect, putting a hypen between go and $VERSION would have read > better, but such is life. If possible I think it's worth using <em> or <b> > or something to encourage readers to no just blindly copy the string into > their command line. > > On second thought it was a mistake to suggest $VERSION, not VERSION. The > former, if copied onto the command line will expand to go.-.tar.gz, but the > latter would be goVERSION.OS-ARCH.tar.gz which would give a cleaner not > found error. > Let's wait for others opinions.
Sign in to reply to this message.
On 2014/01/08 02:29:42, minux wrote: > On Tue, Jan 7, 2014 at 6:39 PM, Dave Cheney <mailto:dave@cheney.net> wrote: > > > In retrospect, putting a hypen between go and $VERSION would have read > > better, but such is life. If possible I think it's worth using <em> or <b> > > or something to encourage readers to no just blindly copy the string into > > their command line. > > > > On second thought it was a mistake to suggest $VERSION, not VERSION. The > > former, if copied onto the command line will expand to go.-.tar.gz, but the > > latter would be goVERSION.OS-ARCH.tar.gz which would give a cleaner not > > found error. > > > Let's wait for others opinions. Releases don't happen very often, and it's nice to be able to copy/paste onto the command line. I'd rather see this get added to the release checklist that Andrew doubtless has, and just update it with every release. -josh
Sign in to reply to this message.
On Wed, Jan 8, 2014 at 2:25 PM, <josharian@gmail.com> wrote: > Releases don't happen very often, and it's nice to be able to copy/paste > onto the command line. I'd rather see this get added to the release > checklist that Andrew doubtless has, and just update it with every > release. > If you have downloaded the binary release, I think it's much easier to use TAB to complete the filename. And we also provide {linux,freebsd,darwin}{386,amd64} tar.gz binary distributions, so we can't hope to provide ready-to-use command line for every case. There is another alternative, export a Version function to godoc templates, and convert doc/install.html to use that function, so that the version is automatically correct for the version of Go that runs golang.org.
Sign in to reply to this message.
R=adg@golang.org (assigned by dave@cheney.net)
Sign in to reply to this message.
I don't know about this. It's nice to be explicit, and not force new users to think harder than they need to. Is it really a big deal to just update the numbers?
Sign in to reply to this message.
https://codereview.appspot.com/48730043/diff/90001/doc/install.html File doc/install.html (right): https://codereview.appspot.com/48730043/diff/90001/doc/install.html#newcode83 doc/install.html:83: The name of the archive may differ, depending on the version of Go you are the meaning of the pseudovariables can be made clear by replacing this sentence (actually the whole paragraph) with: Choose the archive file appropriate for your installation. For instance, if you are installing Go version 1.2 for ARM on Linux, the archive you want is called <code>go1.2.linux-arm.tar.gz</code>.
Sign in to reply to this message.
PTAL. On 2014/01/31 23:04:45, r wrote: > https://codereview.appspot.com/48730043/diff/90001/doc/install.html#newcode83 > doc/install.html:83: The name of the archive may differ, depending on the > version of Go you are > the meaning of the pseudovariables can be made clear by replacing this sentence > (actually the whole paragraph) with: > > Choose the archive file appropriate for your installation. > For instance, if you are installing Go version 1.2 for ARM on Linux, the archive > you want is called <code>go1.2.linux-arm.tar.gz</code>. Because we don't actually distribute linux/arm binary distributions, i changed to freebsd/386.
Sign in to reply to this message.
https://codereview.appspot.com/48730043/diff/100001/doc/install.html File doc/install.html (right): https://codereview.appspot.com/48730043/diff/100001/doc/install.html#newcode84 doc/install.html:84: For instance, if you are installing Go version 1.2 for 386 on FreeBSD, the archive maybe let's choose something people are more likely to use "for 64-bit x86 on Linux" "go1.2.linux-amd64.tar.gz"
Sign in to reply to this message.
PTAL. On Wed, Feb 5, 2014 at 4:55 AM, <adg@golang.org> wrote: > https://codereview.appspot.com/48730043/diff/100001/doc/ > install.html#newcode84 > doc/install.html:84: For instance, if you are installing Go version 1.2 > for 386 on FreeBSD, the archive > maybe let's choose something people are more likely to use > > "for 64-bit x86 on Linux" > "go1.2.linux-amd64.tar.gz" > Done.
Sign in to reply to this message.
LGTM but leave for adg https://codereview.appspot.com/48730043/diff/100001/doc/install.html File doc/install.html (right): https://codereview.appspot.com/48730043/diff/100001/doc/install.html#newcode85 doc/install.html:85: you want is called <code>go1.2.freebsd-386.tar.gz</code>. i agree with adg: why freebsd not linux, which is a far likelier download candidate? i suspect that example will forestall some rookie errors. but it's not a big deal. this is fine.
Sign in to reply to this message.
On 2014/02/06 19:27:09, r wrote: > https://codereview.appspot.com/48730043/diff/100001/doc/install.html#newcode85 > doc/install.html:85: you want is called <code>go1.2.freebsd-386.tar.gz</code>. > i agree with adg: why freebsd not linux, which is a far likelier download > candidate? i suspect that example will forestall some rookie errors. > > but it's not a big deal. this is fine. I adopted adg's advice and the latest changeset uses linux-amd64.
Sign in to reply to this message.
LGTM thanks minux
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=1a3006bc130f *** doc/install.html: get rid of explicit version number in docs. So that we don't need to update the file each time a new version is released. LGTM=dave, r, adg R=golang-codereviews, dave, josharian, gobot, adg, r CC=golang-codereviews https://codereview.appspot.com/48730043
Sign in to reply to this message.
|
