From 20dc1212f8d62f8e7db7eca49abc046ce85811f6 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 14 Mar 2018 16:51:40 -0500 Subject: [PATCH] Address code review (mostly comments) --- diagnostics.go | 37 +++++++++---------- ...cs_test.go => diagnostics_internal_test.go | 2 +- gopsutil/systeminfo.go | 22 +++++------ 3 files changed, 30 insertions(+), 31 deletions(-) rename diagnostics_test.go => diagnostics_internal_test.go (99%) diff --git a/diagnostics.go b/diagnostics.go index 1abcdd144..710f66c2d 100644 --- a/diagnostics.go +++ b/diagnostics.go @@ -38,7 +38,7 @@ type versionResponse struct { Message string `json:"message"` } -// DiagnosticsCollector represents a collector/sender of diagnostics data +// DiagnosticsCollector represents a collector/sender of diagnostics data. type DiagnosticsCollector struct { mu sync.Mutex host string @@ -57,9 +57,8 @@ type DiagnosticsCollector struct { server *Server } -// New returns a pointer to a new DiagnosticsCollector Client given an addr in the format "hostname:port". +// NewDiagnosticsCollector returns a new DiagnosticsCollector given an addr in the format "hostname:port". func NewDiagnosticsCollector(host string) *DiagnosticsCollector { - return &DiagnosticsCollector{ host: host, VersionURL: defaultVersionCheckURL, @@ -118,7 +117,7 @@ func (d *DiagnosticsCollector) CheckVersion() error { return fmt.Errorf("json decode: %s", err) } - // Same a version as last test + // If version has not changed since the last check, return if rsp.Version == d.lastVersion { return nil } @@ -133,8 +132,8 @@ func (d *DiagnosticsCollector) CheckVersion() error { // compareVersion check version strings. func (d *DiagnosticsCollector) compareVersion(value string) error { - currentVersion := VersionSegments(value) - localVersion := VersionSegments(d.version) + currentVersion := versionSegments(value) + localVersion := versionSegments(d.version) if localVersion[0] < currentVersion[0] { //Major return fmt.Errorf("Warning: You are running Pilosa %s. A newer version (%s) is available: https://github.com/pilosa/pilosa/releases", d.version, value) @@ -249,8 +248,8 @@ func (d *DiagnosticsCollector) EnrichWithSchemaProperties() { d.Set("TimeQuantumEnabled", timeQuantumEnabled) } -// VersionSegments returns the numeric segments of the version as a slice of ints. -func VersionSegments(segments string) []int { +// versionSegments returns the numeric segments of the version as a slice of ints. +func versionSegments(segments string) []int { segments = strings.Trim(segments, "v") segments = strings.Split(segments, "-")[0] s := strings.Split(segments, ".") @@ -261,7 +260,7 @@ func VersionSegments(segments string) []int { return segmentSlice } -// SystemInfo collects information about the host OS +// SystemInfo collects information about the host OS. type SystemInfo interface { Uptime() (uint64, error) Platform() (string, error) @@ -273,51 +272,51 @@ type SystemInfo interface { MemUsed() (uint64, error) } -// NewNopSystemInfo creates a no-op implementation of SystemInfo +// NewNopSystemInfo creates a no-op implementation of SystemInfo. func NewNopSystemInfo() *NopSystemInfo { return &NopSystemInfo{} } -// NopSystemInfo is a no-op implementation of SystemInfo +// NopSystemInfo is a no-op implementation of SystemInfo. type NopSystemInfo struct { } -// Uptime is a no-op implementation of SystemInfo.Uptime +// Uptime is a no-op implementation of SystemInfo.Uptime. func (n *NopSystemInfo) Uptime() (uint64, error) { return 0, nil } -// Platform is a no-op implementation of SystemInfo.Platform +// Platform is a no-op implementation of SystemInfo.Platform. func (n *NopSystemInfo) Platform() (string, error) { return "", nil } -// Family is a no-op implementation of SystemInfo.Family +// Family is a no-op implementation of SystemInfo.Family. func (n *NopSystemInfo) Family() (string, error) { return "", nil } -// OSVersion is a no-op implementation of SystemInfo.OSVersion +// OSVersion is a no-op implementation of SystemInfo.OSVersion. func (n *NopSystemInfo) OSVersion() (string, error) { return "", nil } -// KernelVersion is a no-op implementation of SystemInfo.KernelVersion +// KernelVersion is a no-op implementation of SystemInfo.KernelVersion. func (n *NopSystemInfo) KernelVersion() (string, error) { return "", nil } -// MemFree is a no-op implementation of SystemInfo.MemFree +// MemFree is a no-op implementation of SystemInfo.MemFree. func (n *NopSystemInfo) MemFree() (uint64, error) { return 0, nil } -// MemTotal is a no-op implementation of SystemInfo.MemTotal +// MemTotal is a no-op implementation of SystemInfo.MemTotal. func (n *NopSystemInfo) MemTotal() (uint64, error) { return 0, nil } -// MemUsed is a no-op implementation of SystemInfo.MemUsed +// MemUsed is a no-op implementation of SystemInfo.MemUsed. func (n *NopSystemInfo) MemUsed() (uint64, error) { return 0, nil } diff --git a/diagnostics_test.go b/diagnostics_internal_test.go similarity index 99% rename from diagnostics_test.go rename to diagnostics_internal_test.go index 7b66d8e18..eb2498297 100644 --- a/diagnostics_test.go +++ b/diagnostics_internal_test.go @@ -69,7 +69,7 @@ func TestDiagnosticsClient(t *testing.T) { func TestDiagnosticsVersion_Parse(t *testing.T) { version := "0.1.1" - vs := VersionSegments(version) + vs := versionSegments(version) output := []int{0, 1, 1} if !reflect.DeepEqual(vs, output) { diff --git a/gopsutil/systeminfo.go b/gopsutil/systeminfo.go index 433aacc30..e6285ade8 100644 --- a/gopsutil/systeminfo.go +++ b/gopsutil/systeminfo.go @@ -8,14 +8,14 @@ import ( var _ pilosa.SystemInfo = NewSystemInfo() -// SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS +// SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS. type SystemInfo struct { platform string family string osVersion string } -// Uptime returns the system uptime in seconds +// Uptime returns the system uptime in seconds. func (s *SystemInfo) Uptime() (uptime uint64, err error) { hostInfo, err := host.Info() if err != nil { @@ -24,7 +24,7 @@ func (s *SystemInfo) Uptime() (uptime uint64, err error) { return hostInfo.Uptime, nil } -// collectPlatformInfo fetches and caches system platform information +// collectPlatformInfo fetches and caches system platform information. func (s *SystemInfo) collectPlatformInfo() error { var err error if s.platform == "" { @@ -36,7 +36,7 @@ func (s *SystemInfo) collectPlatformInfo() error { return nil } -// Uptime returns the system platform +// Platform returns the system platform. func (s *SystemInfo) Platform() (string, error) { err := s.collectPlatformInfo() if err != nil { @@ -45,7 +45,7 @@ func (s *SystemInfo) Platform() (string, error) { return s.platform, nil } -// Family returns the system family +// Family returns the system family. func (s *SystemInfo) Family() (string, error) { err := s.collectPlatformInfo() if err != nil { @@ -54,7 +54,7 @@ func (s *SystemInfo) Family() (string, error) { return s.family, err } -// OSVersion returns the OS Version +// OSVersion returns the OS Version. func (s *SystemInfo) OSVersion() (string, error) { err := s.collectPlatformInfo() if err != nil { @@ -63,7 +63,7 @@ func (s *SystemInfo) OSVersion() (string, error) { return s.osVersion, err } -// MemFree returns the amount of free memory in bytes +// MemFree returns the amount of free memory in bytes. func (s *SystemInfo) MemFree() (uint64, error) { memInfo, err := mem.VirtualMemory() if err != nil { @@ -72,7 +72,7 @@ func (s *SystemInfo) MemFree() (uint64, error) { return memInfo.Free, err } -// MemFree returns the amount of total memory in bytes +// MemTotal returns the amount of total memory in bytes. func (s *SystemInfo) MemTotal() (uint64, error) { memInfo, err := mem.VirtualMemory() if err != nil { @@ -81,7 +81,7 @@ func (s *SystemInfo) MemTotal() (uint64, error) { return memInfo.Total, err } -// MemFree returns the amount of used memory in bytes +// MemUsed returns the amount of used memory in bytes. func (s *SystemInfo) MemUsed() (uint64, error) { memInfo, err := mem.VirtualMemory() if err != nil { @@ -90,12 +90,12 @@ func (s *SystemInfo) MemUsed() (uint64, error) { return memInfo.Used, err } -// KernelVersion returns the kernel version as a string +// KernelVersion returns the kernel version as a string. func (s *SystemInfo) KernelVersion() (string, error) { return host.KernelVersion() } -// NewSystemInfo is a constructor for the gopsutil implementation of SystemInfo +// NewSystemInfo is a constructor for the gopsutil implementation of SystemInfo. func NewSystemInfo() *SystemInfo { return &SystemInfo{} }