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

Issue 5877059: code review 5877059: misc/dist: updates to installer script (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 11 months ago by adg
Modified:
12 years, 11 months ago
Reviewers:
Joe Poirier
CC:
golang-dev
Visibility:
Public.

Description

misc/dist: updates to installer script Now sets GOROOT. Fixes issue 3287. Fixes issue 3361.

Patch Set 1 #

Total comments: 1

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -25 lines) Patch
M misc/dist/windows/installer.wxs View 7 chunks +25 lines, -25 lines 0 comments Download

Messages

Total messages: 6
adg
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg
12 years, 11 months ago (2012-03-23 00:26:03 UTC) #1
adg
*** Submitted as http://code.google.com/p/go/source/detail?r=0e050c9637fb *** misc/dist: updates to installer script Now sets GOROOT. Fixes issue ...
12 years, 11 months ago (2012-03-23 00:48:59 UTC) #2
adg
Submitting this for the weekly. It works for me. Let me know if I did ...
12 years, 11 months ago (2012-03-23 00:49:44 UTC) #3
Joe Poirier
LGTM with just a single comment http://codereview.appspot.com/5877059/diff/1/misc/dist/windows/installer.wxs File misc/dist/windows/installer.wxs (right): http://codereview.appspot.com/5877059/diff/1/misc/dist/windows/installer.wxs#newcode81 misc/dist/windows/installer.wxs:81: Arguments='/c start "Godoc ...
12 years, 11 months ago (2012-03-23 00:56:17 UTC) #4
adg
On 23 March 2012 11:56, <jdpoirier@gmail.com> wrote: > LGTM > > with just a single ...
12 years, 11 months ago (2012-03-23 01:49:50 UTC) #5
Joe Poirier
12 years, 11 months ago (2012-03-23 02:10:56 UTC) #6
On Thu, Mar 22, 2012 at 8:49 PM, Andrew Gerrand <adg@golang.org> wrote:
> On 23 March 2012 11:56,  <jdpoirier@gmail.com> wrote:
>> LGTM
>>
>> with just a single comment
>>
>>
>> http://codereview.appspot.com/5877059/diff/1/misc/dist/windows/installer.wxs
>> File misc/dist/windows/installer.wxs (right):
>>
>>
http://codereview.appspot.com/5877059/diff/1/misc/dist/windows/installer.wxs#...
>> misc/dist/windows/installer.wxs:81: Arguments='/c start "Godoc Server
>>
>> http://localhost:6060" /d"[INSTALLDIR]bin" godoc.exe -http=:6060
>> -goroot="[INSTALLDIR]" -path="%GOPATH%" &amp;&amp; start
>> http://localhost:6060'
>> this needs to have an open quote at the first start command and an end
>> quote at the end of 6060.  e.g.   '/c "start .... 6060"'   <- hard to
>> see but it's a double and single quote together.
>
> Really? That doesn't make much sense to me. There were some problems
> with the shortcut, though, which should now be fixed.
>
> Andrew

The problem is is that godoc doesn't like the single back slashes in
the goroot path.
The Arguments variable requires a quoted string and the shortcut field
requires everything after the /c to be in quotes.
-joe
Sign in to reply to this message.

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