-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V2 #95
Conversation
d095cb8
to
1ed0150
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for all of this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the work! One tiny ask I would have is to use the full term GraphQL
instead of GQL
, since GQL as a proposal is a different query language than GraphQL.
This PR introduces some API changes that are breaking and thus constitute bumping the major version to 2.0.0. This PR is best viewed by commits as each changes an individual package.
Changes by package:
CurrentRepository
function from top levelgh
package torepository
package. This felt like a more natural place for it.CurrentRepository
function toCurrent
as it is now in therepository
package.Repository
interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it. Allowed for removing ofinternal/repository/repository.go
.GQL
toGraphQL
.RESTClient
function from top levelgh
package toapi
package. This made more sense in theapi
package than at the top level.GQLClient
function from top levelgh
package toapi
package. This made more sense in theapi
package than at the top level.HTTPClient
function from top levelgh
package toapi
package. This made more sense in theapi
package than at the top level.RESTClient
interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it.GQLClient
interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it.ClientOptions
to be used with value semantics instead of pointer semantics. This allowsClientOptions
to be used in multiple clients without having to worry about them being modified.HTTPError
andGQLError
custom error types to take pointers as method receivers. This is a Go convention which should be followed unless good reason not to.DefaultRESTClient
,DefaultHTTPClient
,DefaultGQLClient
convenience functions. With the change to using value semantics for the options these felt nice to have.internal/api
folder.GQLClient
Query
andMutate
methods to returnGQLError
instead ofshurcooL/graphql.Errors
.New
factory function to return*Browser
instead ofBrowser
. All methods onBrowser
use*Browser
as the receiver so we should be returning a pointer.InvalidConfigFileError
andKeyNotFoundError
custom error types to take pointers as method receivers. This is a Go convention which should be followed unless good reason not to.New
factory function to return*Template
instead ofTemplate
. All methods onTemplate
use*Template
as the receiver so we should be returning a pointer.Fixes #97