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

Change example_gh_test.go to package gh_test #87

Closed
mntlty opened this issue Nov 4, 2022 · 5 comments · Fixed by #88
Closed

Change example_gh_test.go to package gh_test #87

mntlty opened this issue Nov 4, 2022 · 5 comments · Fixed by #88

Comments

@mntlty
Copy link
Contributor

mntlty commented Nov 4, 2022

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:

  • the examples cannot be copied as is
  • it can be confusing to Go newcomers, who may not be familiar with how to solve the issue

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

@mntlty
Copy link
Contributor Author

mntlty commented Nov 4, 2022

A concrete implementation is in 114787b in case this makes sense to others

@mislav
Copy link
Contributor

mislav commented Nov 5, 2022

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
Copy link
Contributor Author

mntlty commented Nov 7, 2022

@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
Copy link
Contributor

mislav commented Nov 7, 2022

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

@mntlty
Copy link
Contributor Author

mntlty commented Nov 7, 2022

done!

@mislav mislav closed this as completed in #88 Nov 7, 2022
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 a pull request may close this issue.

2 participants