From ffaa4a4a31e505d333656acd7613035cbe4d64ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lucas=20K=C3=A4ldstr=C3=B6m?= <lucas@weave.works>
Date: Tue, 18 Aug 2020 19:25:13 +0300
Subject: [PATCH] Address review comments

---
 github/auth.go                     |  4 +-
 gitprovider/client_options_test.go | 60 ------------------------------
 gitprovider/errors.go              |  2 +-
 3 files changed, 3 insertions(+), 63 deletions(-)

diff --git a/github/auth.go b/github/auth.go
index 7a3687e0..3205a9de 100644
--- a/github/auth.go
+++ b/github/auth.go
@@ -152,8 +152,8 @@ func WithPreChainTransportHook(preRoundTripperFunc gitprovider.ChainableRoundTri
 	return buildCommonOption(gitprovider.CommonClientOptions{PreChainTransportHook: preRoundTripperFunc})
 }
 
-// WithPostChainTransportHook registers a ChainableRoundTripperFunc "before" the cache and authentication
-// transports in the chain. For more information, see NewClient, and gitprovider.CommonClientOptions.PreChainTransportHook.
+// WithPostChainTransportHook registers a ChainableRoundTripperFunc "after" the cache and authentication
+// transports in the chain. For more information, see NewClient, and gitprovider.CommonClientOptions.WithPostChainTransportHook.
 func WithPostChainTransportHook(postRoundTripperFunc gitprovider.ChainableRoundTripperFunc) ClientOption {
 	// Don't allow an empty value
 	if postRoundTripperFunc == nil {
diff --git a/gitprovider/client_options_test.go b/gitprovider/client_options_test.go
index d74e8e8b..be13dc5a 100644
--- a/gitprovider/client_options_test.go
+++ b/gitprovider/client_options_test.go
@@ -123,66 +123,6 @@ func Test_makeOptions(t *testing.T) {
 			opts:         []commonClientOption{withPostChainTransportHook(dummyRoundTripper1), withPostChainTransportHook(dummyRoundTripper1)},
 			expectedErrs: []error{ErrInvalidClientOptions},
 		},
-		/*{
-			name: "WithDestructiveAPICalls",
-			opts: []ClientOption{WithDestructiveAPICalls(true)},
-			want: buildCommonOption(gitprovider.CommonClientOptions{EnableDestructiveAPICalls: gitprovider.BoolVar(true)}),
-		},
-		{
-			name: "WithPreChainTransportHook",
-			opts: []ClientOption{WithPreChainTransportHook(dummyRoundTripper1)},
-			want: buildCommonOption(gitprovider.CommonClientOptions{PreChainTransportHook: dummyRoundTripper1}),
-		},
-		{
-			name:         "WithPreChainTransportHook, nil",
-			opts:         []ClientOption{WithPreChainTransportHook(nil)},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},
-		{
-			name: "WithPostChainTransportHook",
-			opts: []ClientOption{WithPostChainTransportHook(dummyRoundTripper2)},
-			want: buildCommonOption(gitprovider.CommonClientOptions{PostChainTransportHook: dummyRoundTripper2}),
-		},
-		{
-			name:         "WithPostChainTransportHook, nil",
-			opts:         []ClientOption{WithPostChainTransportHook(nil)},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},
-		{
-			name: "WithOAuth2Token",
-			opts: []ClientOption{WithOAuth2Token("foo")},
-			want: &clientOptions{AuthTransport: oauth2Transport("foo")},
-		},
-		{
-			name:         "WithOAuth2Token, empty",
-			opts:         []ClientOption{WithOAuth2Token("")},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},
-		{
-			name: "WithPersonalAccessToken",
-			opts: []ClientOption{WithPersonalAccessToken("foo")},
-			want: &clientOptions{AuthTransport: patTransport("foo")},
-		},
-		{
-			name:         "WithPersonalAccessToken, empty",
-			opts:         []ClientOption{WithPersonalAccessToken("")},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},
-		{
-			name:         "WithPersonalAccessToken and WithOAuth2Token, exclusive",
-			opts:         []ClientOption{WithPersonalAccessToken("foo"), WithOAuth2Token("foo")},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},
-		{
-			name: "WithConditionalRequests",
-			opts: []ClientOption{WithConditionalRequests(true)},
-			want: &clientOptions{EnableConditionalRequests: gitprovider.BoolVar(true)},
-		},
-		{
-			name:         "WithConditionalRequests, exclusive",
-			opts:         []ClientOption{WithConditionalRequests(true), WithConditionalRequests(false)},
-			expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
-		},*/
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/gitprovider/errors.go b/gitprovider/errors.go
index f14ef4b0..b4332276 100644
--- a/gitprovider/errors.go
+++ b/gitprovider/errors.go
@@ -59,7 +59,7 @@ var (
 	ErrInvalidClientOptions = errors.New("invalid options given to NewClient()")
 	// ErrDestructiveCallDisallowed happens when the client isn't set up with WithDestructiveAPICalls()
 	// but a destructive action is called.
-	ErrDestructiveCallDisallowed = errors.New("a destructive call was blocked because it wasn't allowed by the client")
+	ErrDestructiveCallDisallowed = errors.New("destructive call was blocked, disallowed by client")
 	// ErrInvalidTransportChainReturn is returned if a ChainableRoundTripperFunc returns nil, which is invalid.
 	ErrInvalidTransportChainReturn = errors.New("the return value of a ChainableRoundTripperFunc must not be nil")
 )