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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by Joe Poirier
Modified:
1 year, 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/
2 years ago #1
bradfitz
LGTM What's the result of not setting the default to C:\ ? What else would ...
2 years ago #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 ...
2 years ago #3
Joe Poirier
On 2012/03/24 23:25:10, bradfitz wrote: > LGTM > > What's the result of not setting ...
2 years ago #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: > ...
2 years ago #5
Joe Poirier
If someone could build and test the windows-amd64 version it would be much appreciated. -joe
2 years ago #6
bradfitz
Sorry, I assumed you'd already tested this when I LGTM'd. On Sat, Mar 24, 2012 ...
2 years ago #7
Joe Poirier
On Sat, Mar 24, 2012 at 7:59 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Sorry, I ...
2 years ago #8
bradfitz
On Sat, Mar 24, 2012 at 6:58 PM, Joseph Poirier <jdpoirier@gmail.com> wrote: > On Sat, ...
2 years ago #9
Joe Poirier
On Sat, Mar 24, 2012 at 9:07 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > > > ...
2 years ago #10
Joe Poirier
On Sat, Mar 24, 2012 at 10:04 PM, Joseph Poirier <jdpoirier@gmail.com> wrote: > On Sat, ...
2 years ago #11
adg
LGTM Tested on windows-amd64.
2 years ago #12
adg
2 years ago #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 1278:e6ce13d99bf5