search

cockroachdb / cockroach-go

Packages for go clients.
Apache License 2.0
160 stars 70 forks source link

testserver: Start() is useless #103

Closed ajwerner closed 3 years ago

ajwerner commented 3 years ago

All constructors of the TestServer call Start() before returning. Start() cannot be used to restart a server. This is true of the multi-tenant server too. Why do we have this method? Also, why do we even have an interface for TestServer given there is just one impl?

rafiss commented 3 years ago

Start() is leftover from an old refactor: https://github.com/cockroachdb/cockroach-go/pull/74#pullrequestreview-426425958

But it causes real confusion, as in https://github.com/cockroachdb/cockroach-go/issues/98

ajwerner commented 3 years ago

If we can't just remove it, we should either make it a no-op or return a clearer API to tell the user that the API is bad. There's literally no way to use it correctly today. We're potentially stuck on the API for now because semver. That being said, I'm not really convinced anybody out there cares about a method you definitely cannot call.

If we feel like making a new version tag we could remove both the interface and the method.