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

Issue 1758041: code review 1758041: io/ioutil: add CopyFile

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by Kyle Lemons
Modified:
2 months ago
Reviewers:
eliarconcepcion, adg,
CC:
golang-dev
Visibility:
Public.

Description

io/ioutil: add CopyFile

Patch Set 1 #

Patch Set 2 : code review 1758041: Added CopyFile(from, to string) os.Error #

Total comments: 7

Patch Set 3 : code review 1758041: Added CopyFile(from, to string) os.Error #

Patch Set 4 : code review 1758041: Added CopyFile(from, to string) os.Error #

Total comments: 3

Patch Set 5 : code review 1758041: Added CopyFile(from, to string) os.Error #

Total comments: 1

Patch Set 6 : code review 1758041: Added CopyFile(from, to string) os.Error #

Patch Set 7 : code review 1758041: Added CopyFile(from, to string) os.Error #

Patch Set 8 : code review 1758041: Added CopyFile(from, to string) os.Error #

Total comments: 2

Patch Set 9 : code review 1758041: Added CopyFile(from, to string) os.Error #

Total comments: 10
Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -0 lines) Patch
M src/pkg/io/ioutil/ioutil.go View 1 2 3 4 5 6 7 8 1 chunk +33 lines, -0 lines 7 comments Download
M src/pkg/io/ioutil/ioutil_test.go View 1 2 3 4 5 6 1 chunk +25 lines, -0 lines 3 comments Download

Messages

Total messages: 28
Kyle Lemons
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change.
10 years, 2 months ago (2010-07-08 09:52:00 UTC) #1
adg
http://codereview.appspot.com/1758041/diff/2001/3001 File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/1758041/diff/2001/3001#newcode50 src/pkg/io/ioutil/ioutil.go:50: func CopyFile(fromFile, toFile string) os.Error { s/fromFile/inFilename/ s/toFile/outFilename/ http://codereview.appspot.com/1758041/diff/2001/3001#newcode58 ...
10 years, 2 months ago (2010-07-08 10:38:49 UTC) #2
eds
On Thu, Jul 8, 2010 at 4:52 AM, <etherealflaim@gmail.com> wrote: > Reviewers: golang-dev_googlegroups.com, > > ...
10 years, 2 months ago (2010-07-08 15:16:56 UTC) #3
Kyle Lemons
> While I think this function is a great addition, I'm not convinced > that ...
10 years, 2 months ago (2010-07-09 04:24:37 UTC) #4
Kyle Lemons
Hello golang-dev@googlegroups.com, adg (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2010-07-09 04:45:00 UTC) #5
adg
http://codereview.appspot.com/1758041/diff/13001/14001 File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/1758041/diff/13001/14001#newcode50 src/pkg/io/ioutil/ioutil.go:50: func CopyFile(inFilename, outFilename string) (numbytes int64, err os.Error) { ...
10 years, 2 months ago (2010-07-09 23:20:12 UTC) #6
Kyle Lemons
PTAL. > s/numbytes/written/ Fixed. > We don't need to initialise the return values, as the ...
10 years, 2 months ago (2010-07-10 02:51:25 UTC) #7
peterGo
http://codereview.appspot.com/1758041/diff/21001/22001 File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/1758041/diff/21001/22001#newcode49 src/pkg/io/ioutil/ioutil.go:49: // CopyFile copies the named file's contents, m/atime, and ...
10 years, 2 months ago (2010-07-10 04:37:13 UTC) #8
Kyle Lemons
> This function should not be OS dependent; it is. Are you refering to the ...
10 years, 2 months ago (2010-07-10 04:45:43 UTC) #9
adg
LGTM Passing it on to Rob and Russ to see what they think.
10 years, 2 months ago (2010-07-10 06:03:02 UTC) #10
r
this seemingly trivial operation is actually pretty complex. doing it right will require operating-system dependent ...
10 years, 2 months ago (2010-07-12 19:35:42 UTC) #11
rsc1
On 2010/07/12 19:35:42, r wrote: > this seemingly trivial operation is actually pretty complex. doing ...
10 years, 2 months ago (2010-07-12 20:25:01 UTC) #12
Kyle Lemons
> i'm pretty scared by all the suggestions about mmap > and Windows CopyFile API ...
10 years, 2 months ago (2010-07-13 04:18:02 UTC) #13
Kyle Lemons
Sorry for the delay here, work's been getting in the way. PTAL. As was suggested, ...
10 years, 2 months ago (2010-07-18 21:22:44 UTC) #14
r2
The comment needs to be updated. -rob
10 years, 2 months ago (2010-07-18 22:16:39 UTC) #15
Kyle Lemons
Hello r, rsc, rsc1 (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2010-07-19 00:41:45 UTC) #16
adg
Fix comment http://codereview.appspot.com/1758041/diff/40001/35002 File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/1758041/diff/40001/35002#newcode49 src/pkg/io/ioutil/ioutil.go:49: // CopyFile copies the named file's contents ...
10 years, 2 months ago (2010-07-19 03:59:33 UTC) #17
peterGo
http://codereview.appspot.com/1758041/diff/40001/35002 File src/pkg/io/ioutil/ioutil.go (right): http://codereview.appspot.com/1758041/diff/40001/35002#newcode49 src/pkg/io/ioutil/ioutil.go:49: // CopyFile copies the named file's contents If there ...
10 years, 2 months ago (2010-07-19 04:20:06 UTC) #18
Kyle Lemons
Hello r, rsc, rsc1, adg, PeterGo (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2010-07-19 09:19:11 UTC) #19
r
I remain skeptical this function is worth its weight. Deferring to rsc. http://codereview.appspot.com/1758041/diff/46001/47001 File src/pkg/io/ioutil/ioutil.go ...
10 years, 2 months ago (2010-07-19 20:27:09 UTC) #20
cw
This function seems a bit out of place. Reading and Writing files is something people ...
10 years, 2 months ago (2010-07-22 03:22:45 UTC) #21
rsc1
I think this is fine as long as it is simple. There's still some work ...
10 years ago (2010-08-26 16:56:51 UTC) #22
rsc1
Also please update the CL description to be io/ioutil: add CopyFile there's no need for ...
10 years ago (2010-08-26 16:57:18 UTC) #23
adg
9 years, 11 months ago (2010-10-19 03:50:50 UTC) #24
eliarconcepcion
Alo
2 months ago (2020-07-15 01:55:51 UTC) #25
幹你娘
2 months ago (2020-07-21 04:24:03 UTC) #26
離婚,不敢………夠賤、夠爛,身障 https://codereview.appspot.com/1758041/diff/46001/src/pkg/io/ioutil/ioutil.go File src/pkg/io/ioutil/ioutil.go (right): https://codereview.appspot.com/1758041/diff/46001/src/pkg/io/ioutil/ioutil.go#newcode58 src/pkg/io/ioutil/ioutil.go:58: // Open the output file (mode depends on ...
2 months ago (2020-07-21 04:29:36 UTC) #27
2 months ago (2020-07-21 04:54:14 UTC) #28
京城旅遊雅玟沒工作了……活該死好死賤人
Sign in to reply to this message.

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