Address review comments

Author: luxas

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 {

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) {

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")
 )