code review 6896044: go-tour: convert from HTML to present format

go-tour: convert from HTML to present format

This makes it easier for us to maintain the tour, and puts the tour
sample programs in individual go files that are compilable and
testable.

This also removes the code that shows content specific to the appengine
or local versions, erring on the side of inclusivity. I intend to add
some more appengine-specific stuff in a followup CL.

[adg, 2012-12-21 00:51:04]

Hello golang-dev@googlegroups.com,

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

[adg, 2012-12-21 00:57:05]

For the conversion tool, see this CL: https://codereview.appspot.com/7003043

[adg, 2013-01-08 23:52:32]

Added a couple of reviewers. Would love to get this done.

[francesc, 2013-01-14 11:11:45]

https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go#newcode54
gotour/tour.go:54: no = false
break?

https://codereview.appspot.com/6896044/diff/27077/prog/default-selection.go
File prog/default-selection.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/default-selection.go#n...
prog/default-selection.go:9: tick := time.Tick(1e8)
We should use time.Milliseconds here

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-errors.go
File prog/exercise-errors.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-errors.go#new...
prog/exercise-errors.go:3: import (
No need of parenthesis here

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-http-handlers.go
File prog/exercise-http-handlers.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-http-handlers...
prog/exercise-http-handlers.go:3: import (
No need of parens

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-loops-and-fun...
File prog/exercise-loops-and-functions.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-loops-and-fun...
prog/exercise-loops-and-functions.go:3: import (
No need of parens

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-maps.go
File prog/exercise-maps.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/exercise-maps.go#newcode4
prog/exercise-maps.go:4: "code.google.com/p/go-tour/wc"
No need of parens

https://codereview.appspot.com/6896044/diff/27077/prog/exported-names.go
File prog/exported-names.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/exported-names.go#newc...
prog/exported-names.go:9: fmt.Println(math.pi)
This won't compile, which is exactly what the slide is about. But wouldn't it be
better to write only programs that compile?
Maybe we should comment this out?

https://codereview.appspot.com/6896044/diff/27077/prog/for-is-gos-while.go
File prog/for-is-gos-while.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/for-is-gos-while.go#ne...
prog/for-is-gos-while.go:7: for sum < 1000 {
This and for-continued.go have exactly the same content. Is that intended?

https://codereview.appspot.com/6896044/diff/27077/prog/goroutines.go
File prog/goroutines.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/goroutines.go#newcode10
prog/goroutines.go:10: time.Sleep(100 * time.Millisecond)
No more explaining Gosched! Yay for that!

But, are we sure this will still print the hello and world in order?

https://codereview.appspot.com/6896044/diff/27077/prog/range-and-close.go
File prog/range-and-close.go (right):

https://codereview.appspot.com/6896044/diff/27077/prog/range-and-close.go#new...
prog/range-and-close.go:17: c := make(chan int, 10)
This example would be clearer with an unbuffered chan

https://codereview.appspot.com/6896044/diff/27077/tour.article
File tour.article (right):

https://codereview.appspot.com/6896044/diff/27077/tour.article#newcode14
tour.article:14: 
Are these empty lines expected?

[adg, 2013-01-14 22:32:24]

Thanks for the review, but let's not change any of the tour content in this CL.
That can happen in a follow-up change. All of the prog/ files are automatically
generated by a conversion tool, and should be identical to the code samples in
the existing tour.

I'm going for as close to functionally identical in this CL; we can tidy up the
content in a follow-up CL.

https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go#newcode54
gotour/tour.go:54: no = false
On 2013/01/14 11:11:45, gocampoy wrote:
> break?
returned instead

https://codereview.appspot.com/6896044/diff/27077/tour.article
File tour.article (right):

https://codereview.appspot.com/6896044/diff/27077/tour.article#newcode14
tour.article:14: 
On 2013/01/14 11:11:45, gocampoy wrote:
> Are these empty lines expected?

when you say "these" are you referring to just this one, or are there others?
this one is an error, I can't see the others.

[francesc, 2013-01-14 23:03:07]

Fair enough. I want to modify the tour with the feedback I'm getting from the
life events, so I will do it myself later.

On 2013/01/14 22:32:24, adg wrote:
> Thanks for the review, but let's not change any of the tour content in this
CL.
> That can happen in a follow-up change. All of the prog/ files are
automatically
> generated by a conversion tool, and should be identical to the code samples in
> the existing tour.
> 
> I'm going for as close to functionally identical in this CL; we can tidy up
the
> content in a follow-up CL.
> 
> https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go
> File gotour/tour.go (right):
> 
> https://codereview.appspot.com/6896044/diff/27077/gotour/tour.go#newcode54
> gotour/tour.go:54: no = false
> On 2013/01/14 11:11:45, gocampoy wrote:
> > break?
> returned instead
> 
> https://codereview.appspot.com/6896044/diff/27077/tour.article
> File tour.article (right):
> 
> https://codereview.appspot.com/6896044/diff/27077/tour.article#newcode14
> tour.article:14: 
> On 2013/01/14 11:11:45, gocampoy wrote:
> > Are these empty lines expected?
> 
> when you say "these" are you referring to just this one, or are there others?
> this one is an error, I can't see the others.

[francesc, 2013-01-14 23:03:19]

https://codereview.appspot.com/6896044/diff/27077/tour.article
File tour.article (right):

https://codereview.appspot.com/6896044/diff/27077/tour.article#newcode14
tour.article:14: 
I saw just another one and assumed it was something you could see around.
Actually I could just find that other one. I put a comment on it.

On 2013/01/14 22:32:25, adg wrote:
> On 2013/01/14 11:11:45, gocampoy wrote:
> > Are these empty lines expected?
> 
> when you say "these" are you referring to just this one, or are there others?
> this one is an error, I can't see the others.

https://codereview.appspot.com/6896044/diff/39001/tour.article
File tour.article (right):

https://codereview.appspot.com/6896044/diff/39001/tour.article#newcode778
tour.article:778: 
Another example of the extra blank lines.

[adg, 2013-01-22 23:52:43]

Anyone care to review this? It's not as big as it looks; just review the stuff
outside of prog/.

[rsc, 2013-01-23 02:46:55]

LGTM

I did not look carefully at how you are parsing the templates. Is {/* ... */}
your own kind of thing? I thought the template package used {{/* ... */}}.

[adg, 2013-01-23 06:39:34]

*** Submitted as https://code.google.com/p/go-tour/source/detail?r=f2bc36ab9779
***

go-tour: convert from HTML to present format

This makes it easier for us to maintain the tour, and puts the tour
sample programs in individual go files that are compilable and
testable.

This also removes the code that shows content specific to the appengine
or local versions, erring on the side of inclusivity. I intend to add
some more appengine-specific stuff in a followup CL.

R=campoy, bradfitz, campoy, rsc
CC=golang-dev
https://codereview.appspot.com/6896044

[adg, 2013-01-23 06:43:24]

On 23 January 2013 13:46, <rsc@golang.org> wrote:

> I did not look carefully at how you are parsing the templates. Is {/*
> ... */} your own kind of thing? I thought the template package used {{/*
> ... */}}.
>

I just made a mistake.