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

Issue 6847050: code review 6847050: run.bash: unset GOMAXPROCS before ../test (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by dfc
Modified:
9 years, 7 months ago
Reviewers:
CC:
albert.strasheim, minux1, r, golang-dev
Visibility:
Public.

Description

run.{bash,bat,rc}: unset GOMAXPROCS before ../test test/run.go already executes tests in parallel where possible. An unknown GOMAXPROCS value during the tests is known to cause failures with tests that measure allocations. ref: https://groups.google.com/d/topic/golang-nuts/tgMhFJ3F5WY/discussion

Patch Set 1 #

Patch Set 2 : diff -r d9896259d8e5 https://code.google.com/p/go #

Patch Set 3 : diff -r d9896259d8e5 https://code.google.com/p/go #

Patch Set 4 : diff -r d9896259d8e5 https://code.google.com/p/go #

Total comments: 4

Patch Set 5 : diff -r dc4a3f6ba179 https://code.google.com/p/go #

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -2 lines) Patch
M src/run.bash View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/run.bat View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M src/run.rc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10
dfc
Hello fullung@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 7 months ago (2012-11-14 10:18:58 UTC) #1
albert.strasheim
LGTM This change makes this kind of complete test easier: ./make.bash for i in `seq ...
9 years, 7 months ago (2012-11-14 10:24:12 UTC) #2
albert.strasheim
LGTM This change makes this kind of complete test easier: ./make.bash for i in `seq ...
9 years, 7 months ago (2012-11-14 10:24:12 UTC) #3
minux1
please also change run.bat and run.rc.
9 years, 7 months ago (2012-11-14 10:25:24 UTC) #4
dfc
Will do. Sorry, I always forget about those crazy kids. On Wed, Nov 14, 2012 ...
9 years, 7 months ago (2012-11-14 10:25:53 UTC) #5
dfc
Hello fullung@gmail.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-11-14 10:30:58 UTC) #6
minux1
LGTM. https://codereview.appspot.com/6847050/diff/7001/src/run.rc File src/run.rc (right): https://codereview.appspot.com/6847050/diff/7001/src/run.rc#newcode47 src/run.rc:47: time go run run.go GOMAXPROCS='' time go run ...
9 years, 7 months ago (2012-11-14 10:46:00 UTC) #7
r
LGTM https://codereview.appspot.com/6847050/diff/7001/src/run.bash File src/run.bash (right): https://codereview.appspot.com/6847050/diff/7001/src/run.bash#newcode119 src/run.bash:119: time go run run.go this is fine, but ...
9 years, 7 months ago (2012-11-14 14:33:08 UTC) #8
dfc
Thank you for your comments. PTAL https://codereview.appspot.com/6847050/diff/7001/src/run.bash File src/run.bash (right): https://codereview.appspot.com/6847050/diff/7001/src/run.bash#newcode119 src/run.bash:119: time go run ...
9 years, 7 months ago (2012-11-15 00:23:26 UTC) #9
dfc
9 years, 7 months ago (2012-11-15 00:41:49 UTC) #10
*** Submitted as http://code.google.com/p/go/source/detail?r=c4cc7dcc49cb ***

run.{bash,bat,rc}: unset GOMAXPROCS before ../test

test/run.go already executes tests in parallel where
possible. An unknown GOMAXPROCS value during the tests
is known to cause failures with tests that measure
allocations.

ref: https://groups.google.com/d/topic/golang-nuts/tgMhFJ3F5WY/discussion

R=fullung, minux.ma, r
CC=golang-dev
http://codereview.appspot.com/6847050
Sign in to reply to this message.

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