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

Issue 4029053: code review 4029053: os: implement new Process api (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by brainman
Modified:
9 years, 7 months ago
Reviewers:
CC:
mattn, r, niemeyer, rog, rsc, golang-dev
Visibility:
Public.

Description

os: implement new Process api Fixes issue 1004. Fixes issue 1460.

Patch Set 1 #

Patch Set 2 : code review 4029053: os: implement new Process api #

Total comments: 28

Patch Set 3 : code review 4029053: os: implement new Process api #

Total comments: 1

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

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

Total comments: 22

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

Patch Set 7 : diff -r 75ebb1ad8e98 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -447 lines) Patch
M src/cmd/cgo/util.go View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M src/cmd/godoc/main.go View 1 2 3 1 chunk +5 lines, -4 lines 0 comments Download
M src/pkg/exec/exec.go View 1 2 3 4 chunks +42 lines, -38 lines 0 comments Download
M src/pkg/http/triv.go View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M src/pkg/os/Makefile View 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/os/exec.go View 1 2 3 4 5 3 chunks +27 lines, -34 lines 0 comments Download
M src/pkg/os/exec_unix.go View 1 2 3 4 5 3 chunks +20 lines, -111 lines 0 comments Download
M src/pkg/os/exec_windows.go View 1 2 3 4 5 1 chunk +30 lines, -134 lines 0 comments Download
M src/pkg/os/os_test.go View 1 2 3 3 chunks +7 lines, -5 lines 0 comments Download
M src/pkg/syscall/exec_unix.go View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M src/pkg/syscall/exec_windows.go View 1 2 3 4 3 chunks +6 lines, -19 lines 0 comments Download
M src/pkg/syscall/syscall_windows.go View 1 2 3 4 5 4 chunks +28 lines, -66 lines 0 comments Download
M src/pkg/syscall/zsyscall_windows_386.go View 1 2 3 4 4 chunks +38 lines, -32 lines 0 comments Download

Messages

Total messages: 16
brainman
Hello golang-dev@googlegroups.com, I'd like you to review this change.
9 years, 7 months ago (2011-02-02 04:10:19 UTC) #1
mattn
LGTM cool! On 2011/02/02 04:10:19, brainman wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you ...
9 years, 7 months ago (2011-02-02 04:28:04 UTC) #2
r
http://codereview.appspot.com/4029053/diff/2001/src/pkg/exec/exec.go File src/pkg/exec/exec.go (right): http://codereview.appspot.com/4029053/diff/2001/src/pkg/exec/exec.go#newcode25 src/pkg/exec/exec.go:25: // Process is representing the running command. I thought ...
9 years, 7 months ago (2011-02-02 05:28:23 UTC) #3
brainman
Thank you for review. Sorry for my English, but, I assure, I'm as bad in ...
9 years, 7 months ago (2011-02-02 06:50:35 UTC) #4
brainman
Hello golang-dev@googlegroups.com, mattn, r (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2011-02-02 06:50:47 UTC) #5
niemeyer
http://codereview.appspot.com/4029053/diff/10001/src/pkg/os/exec_windows.go File src/pkg/os/exec_windows.go (right): http://codereview.appspot.com/4029053/diff/10001/src/pkg/os/exec_windows.go#newcode26 src/pkg/os/exec_windows.go:26: func (p *Process) Close() Error { Can we call ...
9 years, 7 months ago (2011-02-02 13:10:13 UTC) #6
rog
how limited are handles as a resource in windows? if there are no strict limits, ...
9 years, 7 months ago (2011-02-02 14:35:06 UTC) #7
brainman
On 2011/02/02 13:10:13, niemeyer wrote: > Can we call this Release rather than Close? > ...
9 years, 7 months ago (2011-02-03 00:47:37 UTC) #8
brainman
Hello golang-dev@googlegroups.com, mattn, r, niemeyer, rog (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2011-02-03 00:47:40 UTC) #9
brainman
On 2011/02/02 14:35:06, rog wrote: > how limited are handles as a resource in windows? ...
9 years, 7 months ago (2011-02-03 01:06:08 UTC) #10
rsc
LGTM http://codereview.appspot.com/4029053/diff/16001/src/pkg/os/exec.go File src/pkg/os/exec.go (right): http://codereview.appspot.com/4029053/diff/16001/src/pkg/os/exec.go#newcode11 src/pkg/os/exec.go:11: // Process stores the information about a process ...
9 years, 7 months ago (2011-02-03 03:31:15 UTC) #11
rog
On 3 February 2011 01:06, <alex.brainman@gmail.com> wrote: > On 2011/02/02 14:35:06, rog wrote: >> >> ...
9 years, 7 months ago (2011-02-03 14:42:12 UTC) #12
brainman
Hello mattn, r, niemeyer, rog, rsc (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2011-02-04 03:11:41 UTC) #13
brainman
Thank you for review. http://codereview.appspot.com/4029053/diff/16001/src/pkg/os/exec.go File src/pkg/os/exec.go (right): http://codereview.appspot.com/4029053/diff/16001/src/pkg/os/exec.go#newcode11 src/pkg/os/exec.go:11: // Process stores the information ...
9 years, 7 months ago (2011-02-04 03:11:58 UTC) #14
brainman
On 2011/02/03 14:42:12, rog wrote: > at the least you should add a finalizer to ...
9 years, 7 months ago (2011-02-04 03:13:23 UTC) #15
brainman
9 years, 7 months ago (2011-02-04 03:41:33 UTC) #16
*** Submitted as http://code.google.com/p/go/source/detail?r=f9b54e1e9841 ***

os: implement new Process api

Fixes issue 1004.
Fixes issue 1460.

R=mattn, r, niemeyer, rog, rsc
CC=golang-dev
http://codereview.appspot.com/4029053
Sign in to reply to this message.

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