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

Issue 5901044: code review 5901044: windows: install fixes (Closed)

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

Description

windows: install fixes * set default installation drive to C:\ * remove Win64 component property

Patch Set 1 #

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

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

Total comments: 2

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

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

Messages

Total messages: 13
Joe Poirier
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
12 years ago (2012-03-24 22:23:55 UTC) #1
bradfitz
LGTM What's the result of not setting the default to C:\ ? What else would ...
12 years ago (2012-03-24 23:25:10 UTC) #2
aram
http://codereview.appspot.com/5901044/diff/4001/misc/dist/windows/installer.wxs File misc/dist/windows/installer.wxs (right): http://codereview.appspot.com/5901044/diff/4001/misc/dist/windows/installer.wxs#newcode54 misc/dist/windows/installer.wxs:54: <SetDirectory Id="INSTALLDIRROOT" Value="C:\"/> Should use %SystemDrive%, it can be ...
12 years ago (2012-03-24 23:32:06 UTC) #3
Joe Poirier
On 2012/03/24 23:25:10, bradfitz wrote: > LGTM > > What's the result of not setting ...
12 years ago (2012-03-25 00:10:33 UTC) #4
Joe Poirier
http://codereview.appspot.com/5901044/diff/4001/misc/dist/windows/installer.wxs File misc/dist/windows/installer.wxs (right): http://codereview.appspot.com/5901044/diff/4001/misc/dist/windows/installer.wxs#newcode54 misc/dist/windows/installer.wxs:54: <SetDirectory Id="INSTALLDIRROOT" Value="C:\"/> On 2012/03/24 23:32:06, aram wrote: > ...
12 years ago (2012-03-25 00:10:48 UTC) #5
Joe Poirier
If someone could build and test the windows-amd64 version it would be much appreciated. -joe
12 years ago (2012-03-25 00:56:25 UTC) #6
bradfitz
Sorry, I assumed you'd already tested this when I LGTM'd. On Sat, Mar 24, 2012 ...
12 years ago (2012-03-25 00:59:40 UTC) #7
Joe Poirier
On Sat, Mar 24, 2012 at 7:59 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Sorry, I ...
12 years ago (2012-03-25 01:58:47 UTC) #8
bradfitz
On Sat, Mar 24, 2012 at 6:58 PM, Joseph Poirier <jdpoirier@gmail.com> wrote: > On Sat, ...
12 years ago (2012-03-25 02:07:15 UTC) #9
Joe Poirier
On Sat, Mar 24, 2012 at 9:07 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > > > ...
12 years ago (2012-03-25 03:04:59 UTC) #10
Joe Poirier
On Sat, Mar 24, 2012 at 10:04 PM, Joseph Poirier <jdpoirier@gmail.com> wrote: > On Sat, ...
12 years ago (2012-03-25 03:38:12 UTC) #11
adg
LGTM Tested on windows-amd64.
12 years ago (2012-03-26 05:48:24 UTC) #12
adg
12 years ago (2012-03-26 05:48:25 UTC) #13
*** Submitted as http://code.google.com/p/go/source/detail?r=c5a91a745639 ***

windows: install fixes

* set default installation drive to C:\
* remove Win64 component property

R=golang-dev, bradfitz, aram
CC=golang-dev
http://codereview.appspot.com/5901044

Committer: Andrew Gerrand <adg@golang.org>
Sign in to reply to this message.

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