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

clean up tsdb for golint #4477

Closed
wants to merge 2 commits into from
Closed

clean up tsdb for golint #4477

wants to merge 2 commits into from

Conversation

linearb
Copy link
Contributor

@linearb linearb commented Oct 16, 2015

An attempt to clean up tsdb for golint for issue #4098.

Not including meta.pg.go since that's generated.

Not sure about the following golint complaints, so left those unchanged:

$ golint tsdb/... | grep -v meta.pb.go
tsdb/functions.go:189:4: can probably use "var a []*rawQueryMapOutput" instead
tsdb/functions.go:229:4: can probably use "var val []float64" instead
tsdb/functions.go:235:4: can probably use "var a []float64" instead
tsdb/engine/tsm1/tsm1.go:85:2: don't use ALL_CAPS in Go names; use CamelCase

(Although did change some apparently unnecessary instances of make() into var.)

@otoolep
Copy link
Contributor

otoolep commented Oct 16, 2015

Thanks @linearb -- we are actually doing some significant refactor in this code right now, so will hold off on merging this for now.

@linearb
Copy link
Contributor Author

linearb commented Oct 16, 2015

Sure thing.

@otoolep
Copy link
Contributor

otoolep commented Oct 27, 2015

@linearb -- our major refactor of this code is complete. Can you rebase? It may be tricky, so you may like to re-do from start.

@linearb
Copy link
Contributor Author

linearb commented Oct 27, 2015

Yes, I can do it.

@linearb
Copy link
Contributor Author

linearb commented Oct 27, 2015

Hmm. Tests not passing consistently.

@linearb
Copy link
Contributor Author

linearb commented Oct 27, 2015

Tests pass now. Not sure why they failed after rebasing yesterday as the only thing
changed today was the commit message. CircleCI problem?

(Also, on some runs ran out of file descriptors locally for TestEngine_WriteCompaction_Concurrent
but raised limit and haven't seen errors since.)

Most of these changes are trivial.

But I made some changes from make() to var (which should be checked),
and left others alone:

golint ./... | fgrep -v meta.pb.go
functions.go:190:4: can probably use "var a []*rawQueryMapOutput" instead
functions.go:230:4: can probably use "var val []float64" instead
functions.go:236:4: can probably use "var a []float64" instead
engine/tsm1/tsm1.go:89:2: don't use ALL_CAPS in Go names; use CamelCase

@otoolep
Copy link
Contributor

otoolep commented Oct 27, 2015

This is great @linearb -- I would like to merge. However the files underneath tsdb/engine/tsm1 are undergoing refactor at the moment. If you could just revert changes those files I will then merge.

Then we can revisit the tsm1 files. I am keen to merge this, I just don't want to stomp on that part of the tree.

@jwilder
Copy link
Contributor

jwilder commented Feb 10, 2016

@linearb Can you rebase? #5196 removed a lot of this code and #5375 did similar changes.

@linearb
Copy link
Contributor Author

linearb commented Feb 11, 2016

ok - I'll have a look.

@toddboom
Copy link
Contributor

Closing this out since a simple rebase is probably no longer feasible.

@toddboom toddboom closed this Apr 11, 2016
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.

None yet

4 participants