code review 5447061: time: fix Plan 9 build for new API

time: fix Plan 9 build for new API

I had to move readFile into sys_$GOOS.go
since syscall.Open takes only two arguments
on Plan 9.

[ality, 2011-12-01 10:07:38]

Hello lucio.dere@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[rsc, 2011-12-01 17:44:30]

On a related note, could someone look into why,
on a Linux or OS X machine,

GOOS=plan9 GOARCH=386 CGO_ENABLED=0 make.bash

fails in runtime?  I would be happy to keep the
Plan 9 code at least building, but this has never
built for me, so there is no good state to preserve.

Russ

[brainman, 2011-12-01 22:48:40]

http://codereview.appspot.com/5447061/diff/5001/src/pkg/time/sys_windows.go
File src/pkg/time/sys_windows.go (right):

http://codereview.appspot.com/5447061/diff/5001/src/pkg/time/sys_windows.go#n...
src/pkg/time/sys_windows.go:18: func readFile(name string) ([]byte, error) {
This function is not used on windows. Please revert change to this file.

[brainman, 2011-12-04 08:06:39]

Unix and windows parts are LGTM now. Thank you.
Sorry, but I don't know enough about plan9 to comment.

Alex

[lucio, 2011-12-07 06:20:28]

I trust Anthony more than I trust myself, but it's along the lines of
what I would have done.  So,

LGTM

I haven't yet explored any of the testing, but I do believe this is
different enough from Unix to deserve special attention regarding
tests.  Anthony, are you looking into this?  I'll be digging into the
test suite later this month, do you have any suggestions?

Lucio.

On 12/4/11, alex.brainman@gmail.com <alex.brainman@gmail.com> wrote:
> Unix and windows parts are LGTM now. Thank you.
> Sorry, but I don't know enough about plan9 to comment.
>
> Alex
>
> http://codereview.appspot.com/5447061/
>

[rsc, 2011-12-07 21:31:51]

Looks pretty good if you eliminate the dependencies.
Thanks.

http://codereview.appspot.com/5447061/diff/2003/src/pkg/time/zoneinfo_plan9.go
File src/pkg/time/zoneinfo_plan9.go (right):

http://codereview.appspot.com/5447061/diff/2003/src/pkg/time/zoneinfo_plan9.g...
src/pkg/time/zoneinfo_plan9.go:11: "strconv"
Please do not import strconv or strings.
Those carry with them dependency trees that
it would be best to avoid in a package as low-level as time.
There are itoa and atoi already in this package.

[ality, 2011-12-11 02:46:30]

https://codereview.appspot.com/5447061/diff/2003/src/pkg/time/zoneinfo_plan9.go
File src/pkg/time/zoneinfo_plan9.go (right):

https://codereview.appspot.com/5447061/diff/2003/src/pkg/time/zoneinfo_plan9....
src/pkg/time/zoneinfo_plan9.go:11: "strconv"
On 2011/12/07 21:31:52, rsc wrote:
> Please do not import strconv or strings.
> Those carry with them dependency trees that
> it would be best to avoid in a package as low-level as time.
> There are itoa and atoi already in this package.

Done.

[rsc, 2011-12-12 21:10:40]

LGTM

[rsc, 2011-12-12 21:12:24]

*** Submitted as http://code.google.com/p/go/source/detail?r=f6048ea55080 ***

time: fix Plan 9 build for new API

I had to move readFile into sys_$GOOS.go
since syscall.Open takes only two arguments
on Plan 9.

R=lucio.dere, rsc, alex.brainman
CC=golang-dev
http://codereview.appspot.com/5447061

Committer: Russ Cox <rsc@golang.org>