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

Maybe remove the panic when creating a new client #174

Closed
uginroot opened this issue Feb 28, 2022 · 3 comments
Closed

Maybe remove the panic when creating a new client #174

uginroot opened this issue Feb 28, 2022 · 3 comments
Labels
4.0 Will address this in the next major release, either because it's a breaking change or major effort.

Comments

@uginroot
Copy link

panic(err) // something really wrong with shopName

How is my code working now:

import (
    goshopify "github.com/bold-commerce/go-shopify"
    "net/url"
)
func NewClient(app goshopify.App, shopName string, token string) (*goshopify.Client, error){
    _, err := url.Parse(shopName)
    
    if err != nil {
        return nil, err
    }

    return app.NewClient(shopName, token), nil
}

I think it would be right:

func (a App) NewClient(shopName, token string, opts ...Option) (*Client, error) {
	return NewClient(a, shopName, token, opts...)
}
func NewClient(app App, shopName, token string, opts ...Option) (*Client, error) {
    baseURL, err := url.Parse(ShopBaseUrl(shopName))
    if err != nil {
        return nil, err
    }
    //.....
}

Or:

func (a App) NewClient(shopName url.URL, token string, opts ...Option) (*Client, error) {
	return NewClient(a, shopName, token, opts...)
}
func NewClient(app App, shopName url.URL, token string, opts ...Option) (*Client, error) {
    baseURL := shopName
    //.....
}
@oliver006 oliver006 added the 4.0 Will address this in the next major release, either because it's a breaking change or major effort. label Apr 3, 2023
@oliver006
Copy link
Collaborator

Should make this change for the upcoming breaking-changes release due to #5

@oliver006
Copy link
Collaborator

Tackling this in #258

@oliver006
Copy link
Collaborator

v4.0.0 was released - this should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 Will address this in the next major release, either because it's a breaking change or major effort.
Projects
None yet
Development

No branches or pull requests

2 participants