code review 4317049: os: Fix MkdirAll("/thisdoesnotexist").

os: Fix MkdirAll("/thisdoesnotexist").

Fixes issue 1637.

[albert.strasheim, 2011-03-28 05:03:31]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rh, 2011-03-28 05:18:21]

Fixes issue 1637?

[albert.strasheim, 2011-03-28 05:26:30]

Ah, so it does.

Should I be submitting this, or should we wait for the bug reporter? I
see they even added a test.

Regards

Albert

On Mon, Mar 28, 2011 at 7:18 AM,  <robert.hencke@gmail.com> wrote:
> Fixes issue 1637?
> http://codereview.appspot.com/4317049/

[rh, 2011-03-28 05:41:56]

> Should I be submitting this, or should we wait for the bug reporter? I
> see they even added a test.

It seems the bug reporter has a rietveld account as msolo.  Maybe we could add
to reviewers and they could weigh in?

[albert.strasheim, 2011-03-28 05:44:19]

Hello msolo

This change fixes the bug you reported at:

http://code.google.com/p/go/issues/detail?id=1637

Would you like to submit your own change or should I go ahead with this one?

Regards

Albert

On 2011/03/28 05:41:56, rh wrote:
> > Should I be submitting this, or should we wait for the bug reporter? I
> > see they even added a test.
> It seems the bug reporter has a rietveld account as msolo.  Maybe we could add
> to reviewers and they could weigh in?

[msolo, 2011-03-28 20:56:26]

Patch away, thanks.

On Sun, Mar 27, 2011 at 10:44 PM, <fullung@gmail.com> wrote:

> Hello msolo
>
> This change fixes the bug you reported at:
>
> http://code.google.com/p/go/issues/detail?id=1637
>
> Would you like to submit your own change or should I go ahead with this
> one?
>
> Regards
>
> Albert
>
> On 2011/03/28 05:41:56, rh wrote:
>
>> > Should I be submitting this, or should we wait for the bug reporter?
>>
> I
>
>> > see they even added a test.
>> It seems the bug reporter has a rietveld account as msolo.  Maybe we
>>
> could add
>
>> to reviewers and they could weigh in?
>>
>
> http://codereview.appspot.com/4317049/
>

[albert.strasheim, 2011-03-29 12:27:01]

PTAL.

Added a slightly modified version of the test included with the issue.

On 2011/03/28 05:03:31, albert.strasheim wrote:
> Hello rsc (cc: mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://go.googlecode.com/hg/

[rsc, 2011-04-02 00:18:11]

Please fix the nit below and then it looks good.

http://codereview.appspot.com/4317049/diff/9001/src/pkg/os/path_test.go
File src/pkg/os/path_test.go (right):

http://codereview.appspot.com/4317049/diff/9001/src/pkg/os/path_test.go#newco...
src/pkg/os/path_test.go:197: defer RemoveAll("/_go_os_test")
a strange last statement.
s/defer //

[albert.strasheim, 2011-04-02 06:00:32]

PTAL.

On 2011/04/02 00:18:11, rsc wrote:
> Please fix the nit below and then it looks good.
> 
> http://codereview.appspot.com/4317049/diff/9001/src/pkg/os/path_test.go
> File src/pkg/os/path_test.go (right):
> 
>
http://codereview.appspot.com/4317049/diff/9001/src/pkg/os/path_test.go#newco...
> src/pkg/os/path_test.go:197: defer RemoveAll("/_go_os_test")
> a strange last statement.
> s/defer //

Fixed.

[rsc, 2011-04-04 19:44:47]

LGTM

[rsc, 2011-04-04 19:45:05]

*** Submitted as d2e05970cc89 ***

os: Fix MkdirAll("/thisdoesnotexist").

Fixes issue 1637.

R=rsc, rh, msolo
CC=golang-dev
http://codereview.appspot.com/4317049

Committer: Russ Cox <rsc@golang.org>