Skip to content
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

Merged
merged 13 commits into from
Apr 16, 2023
Merged

V2 #95

merged 13 commits into from
Apr 16, 2023

Conversation

samcoe
Copy link
Contributor

@samcoe samcoe commented Jan 13, 2023

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:

  • repository
    • Move CurrentRepository function from top level gh package to repository package. This felt like a more natural place for it.
    • Change name of CurrentRepository function to Current as it is now in the repository package.
    • Change Repository interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it. Allowed for removing of internal/repository/repository.go.
  • api
    • Change all references of GQL to GraphQL.
    • Move RESTClient function from top level gh package to api package. This made more sense in the api package than at the top level.
    • Move GQLClient function from top level gh package to api package. This made more sense in the api package than at the top level.
    • Move HTTPClient function from top level gh package to api package. This made more sense in the api package than at the top level.
    • Change RESTClient interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it.
    • Change GQLClient interface type to concrete type. The interface was not serving any purpose as there was only one implementation of it.
    • Change ClientOptions to be used with value semantics instead of pointer semantics. This allows ClientOptions to be used in multiple clients without having to worry about them being modified.
    • Change methods on HTTPError and GQLError custom error types to take pointers as method receivers. This is a Go convention which should be followed unless good reason not to.
    • Added DefaultRESTClient, DefaultHTTPClient, DefaultGQLClient convenience functions. With the change to using value semantics for the options these felt nice to have.
    • These changes allowed for removing of internal/api folder.
    • Change GQLClient Query and Mutate methods to return GQLError instead of shurcooL/graphql.Errors.
  • browser
    • Change New factory function to return *Browser instead of Browser. All methods on Browser use *Browser as the receiver so we should be returning a pointer.
  • config
    • Change methods on InvalidConfigFileError and KeyNotFoundError custom error types to take pointers as method receivers. This is a Go convention which should be followed unless good reason not to.
  • template
    • Change New factory function to return *Template instead of Template. All methods on Template use *Template as the receiver so we should be returning a pointer.

Fixes #97

pkg/repository/repository.go Outdated Show resolved Hide resolved
Copy link
Contributor

@vilmibm vilmibm left a 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!

pkg/repository/repository.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mislav mislav left a 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.

@samcoe samcoe merged commit 9330cac into trunk Apr 16, 2023
9 of 10 checks passed
@samcoe samcoe deleted the v2 branch April 16, 2023 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GQLCLient should return GQLError for query and mutation methods
3 participants