Code Review

README.md

@@ -67,8 +67,8 @@ Each version of the client is tagged and the version is updated accordingly.
 
 To see the list of past versions, run `git tag`.
 
-To release a new version, first ensure that `[ClientVersion](./mongodbatlas/version.go)' is updated 
-(i.e., before running `git push origin vx.y.z`, verify that `ClientVersion=x.y.z` should match the tag being pushed to GitHub)
+To release a new version, first ensure that `[Version](./mongodbatlas/mongodbatlas.go)' is updated 
+(i.e., before running `git push origin vx.y.z`, verify that `Version=x.y.z` should match the tag being pushed to GitHub)
 
 ## Roadmap
 

githooks/pre-push

@@ -3,7 +3,7 @@
 # A simple pre-push hook which ensures that the tag being pushed to GitHub
 # matches the version defined in the VERSION_FILE.
 #
-declare -r VERSION_FILE="mongodbatlas/version.go"
+declare -r VERSION_FILE="mongodbatlas/mongodbatlas.go"
 
 # Determine if a tag is being pushed
 tag=
@@ -22,12 +22,12 @@ done
 if [[ -n "$tag" ]]; then
     echo ""
     echo "Releasing '$tag'..."
-    if ! grep -q "ClientVersion = \"$tag\"" "$VERSION_FILE"; then
+    if ! grep -q "Version = \"$tag\"" "$VERSION_FILE"; then
         echo ""
         echo "ERROR: Mismatch between the release version ('$tag') and '$VERSION_FILE'"
         echo "$VERSION_FILE:"
         echo "---------"
-        grep "ClientVersion = " "$VERSION_FILE"
+        grep "Version = " "$VERSION_FILE"
         echo "---------"
         echo
         exit 1

mongodbatlas/mongodbatlas.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/url"
 	"reflect"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -22,6 +23,13 @@ const (
 	defaultBaseURL = "https://cloud.mongodb.com/api/atlas/v1.0/"
 	jsonMediaType  = "application/json"
 	gzipMediaType  = "application/gzip"
+	libraryName    = "go-mongodbatlas"
+	// Version the version of the current API client
+	Version = "0.4.0" // Should be set to the next version planned to be released
+)
+
+var (
+	userAgent = fmt.Sprintf("%s/%s (%s;%s)", libraryName, Version, runtime.GOOS, runtime.GOARCH)
 )
 
 type Doer interface {
@@ -184,7 +192,7 @@ func NewClient(httpClient *http.Client) *Client {
 
 	baseURL, _ := url.Parse(defaultBaseURL)
 
-	c := &Client{client: httpClient, BaseURL: baseURL, UserAgent: DefaultUserAgent()}
+	c := &Client{client: httpClient, BaseURL: baseURL, UserAgent: userAgent}
 
 	c.APIKeys = &APIKeysServiceOp{Client: c}
 	c.CloudProviderSnapshots = &CloudProviderSnapshotsServiceOp{Client: c}
@@ -262,7 +270,7 @@ func SetBaseURL(bu string) ClientOpt {
 // SetUserAgent is a client option for setting the user agent.
 func SetUserAgent(ua string) ClientOpt {
 	return func(c *Client) error {
-		c.UserAgent = fmt.Sprintf("%s %s", ua, DefaultUserAgent())
+		c.UserAgent = fmt.Sprintf("%s %s", ua, userAgent)
 		return nil
 	}
 }

mongodbatlas/mongodbatlas_test.go

@@ -93,8 +93,8 @@ func testClientDefaultBaseURL(t *testing.T, c *Client) {
 }
 
 func testClientDefaultUserAgent(t *testing.T, c *Client) {
-	if c.UserAgent != DefaultUserAgent() {
-		t.Errorf("NewClient UserAgent = %v, expected %v", c.UserAgent, DefaultUserAgent())
+	if c.UserAgent != userAgent {
+		t.Errorf("NewClient UserAgent = %v, expected %v", c.UserAgent, userAgent)
 	}
 }
 
@@ -164,9 +164,9 @@ func TestNewRequest_withUserData(t *testing.T) {
 	}
 
 	// test default user-agent is attached to the request
-	userAgent := req.Header.Get("User-Agent")
-	if c.UserAgent != userAgent {
-		t.Errorf("NewRequest() User-Agent = %v, expected %v", userAgent, c.UserAgent)
+	uA := req.Header.Get("User-Agent")
+	if c.UserAgent != uA {
+		t.Errorf("NewRequest() User-Agent = %v, expected %v", uA, c.UserAgent)
 	}
 }
 
@@ -177,7 +177,7 @@ func TestNewRequest_badURL(t *testing.T) {
 }
 
 func TestNewRequest_withCustomUserAgent(t *testing.T) {
-	ua := fmt.Sprintf("testing/%s", ClientVersion)
+	ua := fmt.Sprintf("testing/%s", Version)
 	c, err := New(nil, SetUserAgent(ua))
 
 	if err != nil {
@@ -186,7 +186,7 @@ func TestNewRequest_withCustomUserAgent(t *testing.T) {
 
 	req, _ := c.NewRequest(ctx, http.MethodGet, "/foo", nil)
 
-	expected := fmt.Sprintf("%s %s", ua, DefaultUserAgent())
+	expected := fmt.Sprintf("%s %s", ua, userAgent)
 	if got := req.Header.Get("User-Agent"); got != expected {
 		t.Errorf("New() UserAgent = %s; expected %s", got, expected)
 	}
@@ -227,7 +227,7 @@ func TestNewGZipRequest_emptyBody(t *testing.T) {
 }
 
 func TestNewGZipRequest_withCustomUserAgent(t *testing.T) {
-	ua := fmt.Sprintf("testing/%s", ClientVersion)
+	ua := fmt.Sprintf("testing/%s", Version)
 	c, err := New(nil, SetUserAgent(ua))
 
 	if err != nil {
@@ -236,7 +236,7 @@ func TestNewGZipRequest_withCustomUserAgent(t *testing.T) {
 
 	req, _ := c.NewGZipRequest(ctx, http.MethodGet, "/foo")
 
-	expected := fmt.Sprintf("%s %s", ua, DefaultUserAgent())
+	expected := fmt.Sprintf("%s %s", ua, userAgent)
 	if got := req.Header.Get("User-Agent"); got != expected {
 		t.Errorf("New() UserAgent = %s; expected %s", got, expected)
 	}
@@ -268,9 +268,9 @@ func TestNewGZipRequest(t *testing.T) {
 	}
 
 	// test default user-agent is attached to the request
-	userAgent := req.Header.Get("User-Agent")
-	if c.UserAgent != userAgent {
-		t.Errorf("NewGZipRequest() User-Agent = %v, expected %v", userAgent, c.UserAgent)
+	uA := req.Header.Get("User-Agent")
+	if c.UserAgent != uA {
+		t.Errorf("NewGZipRequest() User-Agent = %v, expected %v", uA, c.UserAgent)
 	}
 }
 
@@ -450,14 +450,14 @@ func TestDo_completion_callback(t *testing.T) {
 }
 
 func TestCustomUserAgent(t *testing.T) {
-	ua := fmt.Sprintf("testing/%s", ClientVersion)
+	ua := fmt.Sprintf("testing/%s", Version)
 	c, err := New(nil, SetUserAgent(ua))
 
 	if err != nil {
 		t.Fatalf("New() unexpected error: %v", err)
 	}
 
-	expected := fmt.Sprintf("%s %s", ua, DefaultUserAgent())
+	expected := fmt.Sprintf("%s %s", ua, userAgent)
 	if got := c.UserAgent; got != expected {
 		t.Errorf("New() UserAgent = %s; expected %s", got, expected)
 	}