roachprod: remove ClusterImpl interface #72887
Conversation
|
This change is |
RaduBerinde
force-pushed
the
roachprod-no-iface
branch
from
6e5ff93 to
ffb94ed
Compare
There was a problem hiding this comment.
Woo, this is so much cleaner and I bet easier for Goland to resolve definitions. LGTM, you just forgot to change h to c in one spot.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @healthy-pod, @RaduBerinde, and @stevendanna)
pkg/roachprod/install/cockroach.go, line 715 at r1 (raw file):
} func (h *SyncedCluster) getEnvVars() []string {
The linter says s/h/c ;)
The ClusterImpl interface was useful when roachprod had some (very limited) support for Cassandra, but that has been removed. We're left with an unnecessary indirection that lacks documentation. The code doesn't follow any layering, with `Cockroach` working directly with `SyncedCluster` internals and plenty of cockroach-specific code in `SyncedCluster`. This commit removes this interface and improves the documentation of the methods that are simplified. Release note: None
RaduBerinde
force-pushed
the
roachprod-no-iface
branch
from
ffb94ed to
e6c2add
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/roachprod/install/cockroach.go, line 715 at r1 (raw file):
Previously, rail (Rail Aliiev) wrote…
The linter says
s/h/c;)
Removed the receiver altogether.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:
|
bors r+ |
|
Build succeeded: |
The ClusterImpl interface was useful when roachprod had some (very
limited) support for Cassandra, but that has been removed. We're left
with an unnecessary indirection that lacks documentation. The code
doesn't follow any layering, with
Cockroachworking directly withSyncedClusterinternals and plenty of cockroach-specific code inSyncedCluster.This commit removes this interface and improves the documentation of
the methods that are simplified.
Release note: None