Repositories

<ojarjur@google.com>
[email protected] wants to merge refs/heads/ojarjur/skeleton into refs/heads/master

Initial skeleton... still a W.I.P.

<amshali@google.com>
[email protected] commented on 2015-04-21 17:24:08 with status ℹ️

Please add a makefile for building this.

<ojarjur@google.com>
[email protected] commented on 2015-04-21 17:33:45 with status ℹ️

The lack of a Makefile is intentional.\n\nFirst off, the idiomatic way of building go code is with "go build ...", so requiring make in addition to that is not idiomatic. Secondly, requiring "go build" keeps us honest, and prevents us from adding something that breaks the ability to install the tool using "go get".

<amshali@google.com>
[email protected] commented on 2015-04-21 17:45:21 with status ℹ️

Please add some comments to commands list request and review. Otherwise lgtm.

<amshali@google.com>
[email protected] commented on 2015-04-21 17:59:03 with status 👍
<amshali@google.com>
[email protected] commented on 2015-04-21 17:59:03 with status ℹ️

I'll go through this code review again later but if you want to merge it, please go ahead.

<bstanley@google.com>
[email protected] commented on 2015-04-21 18:29:26 with status ℹ️

My only feedback is: a few more comments would be helpful. \nNote: I am not familiar with the Go programming language. Thanks for the intro!\n\nBarbara

<bstanley@google.com>
[email protected] commented on 2015-04-21 18:29:26 with status ℹ️
Location { commit: Some("b1ba6587e1f439ebb856d936720165860cf5cf72"), path: Some("src/repository/git.go"), range: Some(Range { start_line: Some(46) }) }

I like these one-liner, concise comments describing the function. I wish most of the function had this -- especially in the 1st files.

<bstanley@google.com>
[email protected] commented on 2015-04-21 18:29:26 with status ℹ️
Location { commit: Some("b1ba6587e1f439ebb856d936720165860cf5cf72"), path: Some("src/git-review.go"), range: Some(Range { start_line: Some(17) }) }

I like this summary at the top of the file. How about for the other files as well?

<ojarjur@google.com>
[email protected] commented on 2015-04-21 20:55:01 with status ℹ️
Location { commit: Some("b1ba6587e1f439ebb856d936720165860cf5cf72"), path: Some("src/git-review.go"), range: Some(Range { start_line: Some(17) }) }

Those are package godoc, and are essentially the documentation for the containing directory.\n\nThe golang idiom seems to be to only have such a doc in exactly one file (since it describes the directory rather than the file), and then to document the contents of the other files, rather than the files themselves.