-
Notifications
You must be signed in to change notification settings - Fork 115
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
Clean up godoc #82
Clean up godoc #82
Conversation
generate/generate_test.go
Outdated
@@ -204,7 +204,7 @@ func TestGenerateWithConfig(t *testing.T) { | |||
config := test.config | |||
t.Run(test.name, func(t *testing.T) { | |||
err := config.ValidateAndFillDefaults( | |||
filepath.Join(dataDir, test.fakeConfigFilename)) | |||
filepath.Join(dataDir, config.baseDir)) |
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.
config.baseDir is empty at this point? (Also, this value is used to set baseDir, so it seems odd to use baseDir in this context.) I think this can just be dataDir.
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.
Yeah this is kind of sneaky: I'm setting it in some cases above. It's probably better to just have a field in the test-case struct for that; I'll make that change.
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.
All seems reasonable to me.
acd5c80
to
9de1cf4
Compare
21b983b
to
06eeae3
Compare
Before open-sourcing, we want to make sure that (a) GoDoc looks reasonable, and (b) everything in the API is something we want to commit to. In this commit, I do some miscellaneous cleanup on both fronts; this does involve a few breaking changes to the programmatic API (better now than once it has users). In future commits, I'll likely move the documentation for `genqlient.yaml` and `@genqlient` to clearer places, and make `GenqlientDirective` private, such that GoDoc is really only for programmatic users. Fixes #25. Issue: #25 Test plan: make check Reviewers: marksandstrom, miguel, adam
06eeae3
to
a43f52d
Compare
Summary:
Before open-sourcing, we want to make sure that (a) GoDoc looks
reasonable, and (b) everything in the API is something we want to commit
to. In this commit, I do some miscellaneous cleanup on both fronts;
this does involve a few breaking changes to the programmatic API (better
now than once it has users). In future commits, I'll likely move the
documentation for
genqlient.yaml
and@genqlient
to clearer places,and make
GenqlientDirective
private, such that GoDoc is really onlyfor programmatic users.
Fixes #25.
Issue: #25
Test plan:
make check