Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor code organization, clean up, add go.mod file #8

Merged
merged 6 commits into from Jul 5, 2019

Conversation

Projects
None yet
2 participants
@dmitshur
Copy link
Member

commented Jun 22, 2019

This PR consists of 6 commits, each being a smaller logical change (if this were Gerrit, there'd be 6 separate CLs). See individual commit messages for descriptions.

dmitshur added some commits Jun 22, 2019

.travis.yml: use go vet instead of go tool vet
As of Go 1.12, go tool vet is no longer supported.¹
Start using the go vet command instead.

Use RCS format for gofmt diff. It results in cleaner,
easier to read gofmt diffs.

¹ https://golang.org/doc/go1.12#vet
rewrite validateID test from example form to real test
It's more work to write, but it's a better practice and scales better.

@dmitshur dmitshur requested review from hajimehoshi and dominikh Jun 22, 2019

@dmitshur dmitshur force-pushed the modernize branch from 0efdf48 to 960af86 Jun 24, 2019

@hajimehoshi
Copy link
Member

left a comment

Looks like this includes these changes:

  • Add go.mod
  • Use context.Context
  • Other refactoring
    Wouldn't it be possible to split them?
Show resolved Hide resolved main.go
Show resolved Hide resolved main.go
@hajimehoshi

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

This PR consists of 6 commits, each being a smaller logical change

Sorry I missed the comments, but I'm not used to see one PR has multiple changes... (I'm used to Gerrit way :-)

Show resolved Hide resolved main.go Outdated
Show resolved Hide resolved store.go Outdated
Show resolved Hide resolved store.go Outdated
Show resolved Hide resolved main.go Outdated
Show resolved Hide resolved main.go

dmitshur added some commits Jun 22, 2019

refactor top-level code organization
Make changes to improve readability and modernize top-level
code organization.

This change eleminates global variables to help readability.
There is now a snippet store abstraction, and an HTTP server
abstraction. Both are backed by struct types with methods.
As a result, it's easier to see all of the dependencies for
each component of code.
add go.mod and go.sum files
Start building in module mode.
require storage directory to exist, instead of creating
It's less surprising behavior to get an error that the storage
directory does not exist, instead of it being created automatically.
add 3 minute timeout to load and serve snippets
Snippets are expected to be small and should be relatively quick
to fetch and serve. Add a 3 minute timeout as a sanity check.

@dmitshur dmitshur force-pushed the modernize branch from 960af86 to eb3ae24 Jun 30, 2019

@dmitshur
Copy link
Member Author

left a comment

Done, PTAL.

Show resolved Hide resolved main.go Outdated
Show resolved Hide resolved main.go Outdated
Show resolved Hide resolved store.go Outdated
Show resolved Hide resolved store.go Outdated
@hajimehoshi
Copy link
Member

left a comment

lgtm, thanks!

@dmitshur dmitshur merged commit eb3ae24 into master Jul 5, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dmitshur dmitshur deleted the modernize branch Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.