code review 6813107: go.talks/2012/simple.slide: some fixes

go.talks/2012/simple.slide: some fixes

[aam, 2012-11-09 08:02:30]

https://codereview.appspot.com/6813107/diff/3/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode236
2012/simple.slide:236: The testing package also supports benchmarks.
`testing` here too.

https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode250
2012/simple.slide:250: The testing package also supports testable examples.
`testing`

[minux1, 2012-11-09 08:05:51]

Thank you.

On Fri, Nov 9, 2012 at 4:02 PM, <mirtchovski@gmail.com> wrote:
>
> https://codereview.appspot.**com/6813107/diff/3/2012/**
>
simple.slide#newcode236<https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode236>
> 2012/simple.slide:236: The testing package also supports benchmarks.
> `testing` here too.
>
> https://codereview.appspot.**com/6813107/diff/3/2012/**
>
simple.slide#newcode250<https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode250>
> 2012/simple.slide:250: The testing package also supports testable
> examples.
> `testing`
>
both fixed.

[aam, 2012-11-09 08:07:36]

LGTM for package names and spell check.

On Fri, Nov 9, 2012 at 1:05 AM, minux <minux.ma@gmail.com> wrote:
> Thank you.
>
> On Fri, Nov 9, 2012 at 4:02 PM, <mirtchovski@gmail.com> wrote:
>>
>> https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode236
>> 2012/simple.slide:236: The testing package also supports benchmarks.
>> `testing` here too.
>>
>> https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode250
>> 2012/simple.slide:250: The testing package also supports testable
>> examples.
>> `testing`
>
> both fixed.

[minux1, 2012-11-09 08:29:50]

Hello adg@golang.org, mirtchovski@gmail.com, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

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

[minux1, 2012-11-09 08:34:49]

the only new change in Patch Set 4 is this change,
with rationales below.

https://codereview.appspot.com/6813107/diff/4/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/4/2012/simple.slide#newcode268
2012/simple.slide:268: - prof: CPU and memory profiling
i think this should be pprof, because cmd/prof is
not even bundled in release builds (and prof has
been broken on some systems for some time).

if it's pprof, then i also recommend mentioning
runtime/pprof (perhaps net/http/pprof), and
other supported profiling options, such as
goroutine creation, thread creation, and the
most amazing one, blocking profile.

[adg, 2012-11-09 23:39:19]

LGTM thanks
On 9 Nov 2012 09:07, "andrey mirtchovski" <mirtchovski@gmail.com> wrote:

> LGTM for package names and spell check.
>
> On Fri, Nov 9, 2012 at 1:05 AM, minux <minux.ma@gmail.com> wrote:
> > Thank you.
> >
> > On Fri, Nov 9, 2012 at 4:02 PM, <mirtchovski@gmail.com> wrote:
> >>
> >>
> https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode236
> >> 2012/simple.slide:236: The testing package also supports benchmarks.
> >> `testing` here too.
> >>
> >>
> https://codereview.appspot.com/6813107/diff/3/2012/simple.slide#newcode250
> >> 2012/simple.slide:250: The testing package also supports testable
> >> examples.
> >> `testing`
> >
> > both fixed.
>

[r, 2012-11-10 01:00:28]

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode127
2012/simple.slide:127: 
single-file or maybe program in a single source file.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode130
2012/simple.slide:130: Build and install the package in the current directory
(and its dependencies):
the parenthetical remark is in the wrong place. we don't install the
dependencies as a binary; this is confusing.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode134
2012/simple.slide:134: Build and install the `fmt` package (and its
dependencies):
ditto, sorta. you might need another slide.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode138
2012/simple.slide:138: It also acts as an interface for most of the Go tools.
what is 'it'?

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode145
2012/simple.slide:145: Package import paths mirror the code's location on the
file system:
s/on/in/

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode166
2012/simple.slide:166: The `go` tool also fetches Go code from remote
repositories.
didn't we see it just do that?

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode188
2012/simple.slide:188: No prescribed format, just regular comments that precede
the declaration they document.
Comments need no special format, they just need to precede what they document.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode261
2012/simple.slide:261: The example is displayed in godoc alongside the thing it
demonstrates:
`godoc`

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode267
2012/simple.slide:267: - vet: checks code for common programmer mistakes
`vet` etc.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode275
2012/simple.slide:275: * webfront
s/w/W/

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode277
2012/simple.slide:277: webfront is an HTTP server and reverse proxy.
`Webfront`

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode343
2012/simple.slide:343: The `Server` integration test uses the `httptest` package
to construct a dummy HTTP server, synthesizes a set of rules, and constructs a
`Server` instance that use those rules.
s/use/uses/

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode349
2012/simple.slide:349: Each test cases in the table specifies a request URL and
the expected response code and body.
s/cases/case/

[r, 2012-11-10 01:00:52]

if you want to check this in and have adg respond to my comments, LGTM

[minux1, 2012-11-11 17:00:11]

I changed all what I can fix.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode127
2012/simple.slide:127: 
On 2012/11/10 01:00:28, r wrote:
> single-file or maybe program in a single source file.
changed to single-file.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode130
2012/simple.slide:130: Build and install the package in the current directory
(and its dependencies):
On 2012/11/10 01:00:28, r wrote:
> the parenthetical remark is in the wrong place. we don't install the
> dependencies as a binary; this is confusing.
seems this sentence is ambiguous, is "in the curr. dir." refer to the
package or the installation location? (i know it's the former.)

I don't know how to fix this, leave for adg.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode138
2012/simple.slide:138: It also acts as an interface for most of the Go tools.
On 2012/11/10 01:00:28, r wrote:
> what is 'it'?
s/it/This tool/. Not sure if it reads right here.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode145
2012/simple.slide:145: Package import paths mirror the code's location on the
file system:
On 2012/11/10 01:00:28, r wrote:
> s/on/in/
done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode188
2012/simple.slide:188: No prescribed format, just regular comments that precede
the declaration they document.
On 2012/11/10 01:00:28, r wrote:
> Comments need no special format, they just need to precede what they document.
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode261
2012/simple.slide:261: The example is displayed in godoc alongside the thing it
demonstrates:
On 2012/11/10 01:00:28, r wrote:
> `godoc`
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode267
2012/simple.slide:267: - vet: checks code for common programmer mistakes
On 2012/11/10 01:00:28, r wrote:
> `vet` etc.
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode275
2012/simple.slide:275: * webfront
On 2012/11/10 01:00:28, r wrote:
> s/w/W/
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode277
2012/simple.slide:277: webfront is an HTTP server and reverse proxy.
On 2012/11/10 01:00:28, r wrote:
> `Webfront` 
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode343
2012/simple.slide:343: The `Server` integration test uses the `httptest` package
to construct a dummy HTTP server, synthesizes a set of rules, and constructs a
`Server` instance that use those rules.
On 2012/11/10 01:00:28, r wrote:
> s/use/uses/
Done.

https://codereview.appspot.com/6813107/diff/8001/2012/simple.slide#newcode349
2012/simple.slide:349: Each test cases in the table specifies a request URL and
the expected response code and body.
On 2012/11/10 01:00:28, r wrote:
> s/cases/case/
Done.

[minux1, 2012-11-11 17:03:32]

Hi andrew,
  For you convenience, I replicated all rob's comments that I can't
fix onto the newest patch set.

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode130
2012/simple.slide:130: Build and install the package in the current directory
(and its dependencies):
On 2012/11/11 17:00:11, minux wrote:
< On 2012/11/10 01:00:28, r wrote:
< < the parenthetical remark is in the wrong place. we don't install the
< < dependencies as a binary; this is confusing.
< seems this sentence is ambiguous, is "in the curr. dir." refer to the
< package or the installation location? (i know it's the former.) 
< I don't know how to fix this, leave for adg.

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode134
2012/simple.slide:134: Build and install the `fmt` package (and its
dependencies):
On 2012/11/10 01:00:28, r wrote:
< ditto, sorta. you might need another slide.

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode166
2012/simple.slide:166: The `go` tool also fetches Go code from remote
repositories.
On 2012/11/10 01:00:28, r wrote:
< didn't we see it just do that?

[r, 2012-11-11 18:05:24]

LGTM
back to adg

[adg, 2012-11-12 11:10:47]

LGTM

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide
File 2012/simple.slide (right):

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode130
2012/simple.slide:130: Build and install the package in the current directory
(and its dependencies):
On 2012/11/11 17:03:32, minux wrote:
> On 2012/11/11 17:00:11, minux wrote:
> < On 2012/11/10 01:00:28, r wrote:
> < < the parenthetical remark is in the wrong place. we don't install the
> < < dependencies as a binary; this is confusing.
> < seems this sentence is ambiguous, is "in the curr. dir." refer to the
> < package or the installation location? (i know it's the former.) 
> < I don't know how to fix this, leave for adg.

We build and install the package, which could be a library or a binary. I think
this is fine.

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode134
2012/simple.slide:134: Build and install the `fmt` package (and its
dependencies):
On 2012/11/11 17:03:32, minux wrote:
> On 2012/11/10 01:00:28, r wrote:
> < ditto, sorta. you might need another slide.

I'm happy to leave this for now.

https://codereview.appspot.com/6813107/diff/11001/2012/simple.slide#newcode166
2012/simple.slide:166: The `go` tool also fetches Go code from remote
repositories.
On 2012/11/11 17:03:32, minux wrote:
> On 2012/11/10 01:00:28, r wrote:
> < didn't we see it just do that?

No, we didn't actually.

[adg, 2012-11-12 11:42:38]

*** Submitted as
http://code.google.com/p/go/source/detail?r=3f48a5b348c9&repo=talks ***

go.talks/2012/simple.slide: some fixes

R=adg, mirtchovski, minux.ma, r
CC=golang-dev
http://codereview.appspot.com/6813107

Committer: Andrew Gerrand <adg@golang.org>