cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

Change example_gh_test.go to package `gh_test` #87

Closed mntlty closed 1 year ago

mntlty commented 1 year ago

Being in the same package, the example test file can refer to functions such as RESTClient and Exec as local, as opposed to gh.RESTClient and gh.Exec. This presents some challenges:

I would like to suggest changing the package of the example file and updating the functions in it.

mntlty commented 1 year ago

A concrete implementation is in https://github.com/cli/go-gh/commit/114787b4c315071b4b7c8f2cb95256da09e44f3b in case this makes sense to others

mislav commented 1 year ago

Thanks for bringing this up. Will the examples still get rendered of on the main page of generated docs? If so, I'm in favor of this 👍

mntlty commented 1 year ago

@mislav I ran godoc -http=:6060 on the example branch, and it renders the examples while adding the gh package. Comparing local output for http://localhost:6060/pkg/github.com/cli/go-gh/#example_RESTClient_advanced

From trunk:

opts := api.ClientOptions{
    Host:      "github.com",
    AuthToken: "xxxxxxxxxx", // Replace with valid auth token.
    Headers:   map[string]string{"Time-Zone": "America/Los_Angeles"},
    Log:       os.Stdout,
}
client, err := RESTClient(&opts)
if err != nil {
    log.Fatal(err)
}
response := []struct{ Name string }{}
err = client.Get("repos/cli/cli/tags", &response)
if err != nil {
    log.Fatal(err)
}
fmt.Println(response)

From the example branch:

opts := api.ClientOptions{
    Host:      "github.com",
    AuthToken: "xxxxxxxxxx", // Replace with valid auth token.
    Headers:   map[string]string{"Time-Zone": "America/Los_Angeles"},
    Log:       os.Stdout,
}
client, err := gh.RESTClient(&opts)
if err != nil {
    log.Fatal(err)
}
response := []struct{ Name string }{}
err = client.Get("repos/cli/cli/tags", &response)
if err != nil {
    log.Fatal(err)
}
fmt.Println(response)

is there something else I should check for?

mislav commented 1 year ago

Nice, thanks for checking! Please send a PR for this 🙇

mntlty commented 1 year ago

done!