Skip to content

Conversation

dmitshur
Copy link
Member

Please see individual commit messages (expanded versions) for full details.

@dominikh
Copy link
Member

@shurcooL is making me say "LGTM".


It uses the same mapping scheme as the Go Playground, and when a snippet isn't found locally,
it defers to fetching it from the Go Playground. This effectively augments our world of available
snippets with that the Go Playground.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with that of

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching, fixed in 3f9e76b.

@dmitshur
Copy link
Member Author

PTAL.

@neelance
Copy link
Member

LGTM, except that I would prefer to use CircleCI instead of Travis. GopherJS itself is already using CircleCI and it is also free for open source projects.

@dmitshur
Copy link
Member Author

Ok. We can discuss which CI system to use in the future. Let's start with Travis, since I already have a capable setup for it, and if we decide it's worth to switch to Circle, that can be another PR (where we can make sure all of the current functionality is preserved, etc.).

Idiomatic Go style suggests using consistent case for acronyms. Source:
https://github.com/golang/go/wiki/CodeReviewComments#initialisms.

Resolves the following golint issues:

	$ golint .
	id.go:11:6: func snippetBodyToId should be snippetBodyToID
	id.go:25:6: func validateId should be validateID
I mistakenly used to think that Go style suggests always starting
package summary of commands with "Command {name} ...". That isn't true.
Go style only suggests that package summaries should start with
"Package {name} ..." for packages (not commands). Commands have no
restrictions on package summary phrasing. So, remove the "Command"
prefix since it's not helpful. Also remove trailing blank line in
package summary.

Update README to include the same package summary as in godoc.

Add Travis CI with build, diff, vet and tests.

Add badges for godoc and Travis CI to README. Also add installation
instructions. The README was updated in a tool-assisted manner via
gorepogen.
@dmitshur dmitshur force-pushed the lint-style-CI-README-improvements branch from 3f9e76b to c9420af Compare August 16, 2015 00:00
@dmitshur dmitshur merged commit c9420af into master Aug 16, 2015
@dmitshur dmitshur deleted the lint-style-CI-README-improvements branch August 16, 2015 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants