## Summary:
I've felt for a while that types.go is way too confusing, and as I
started to implement some of the more complex cases of generating
interface-types, the cracks were really starting to show. Luckily, I
also finally realized how to fix it: we need to separate the process of
traversing the GraphQL operation and schema to decide what types to
generate from the process of actually generating those types. This
requires an extra set of intermediate data structures, but I think it
makes things quite a lot easier to understand -- and, importantly, it
means that the code-generation doesn't need to go in the order we
traverse the query/schema.
So in this commit, I did that huge refactor. It's probably best to just
review types.go and traverse.go as if they were new; the old code was
quite hard to understand and the new code will hopefully make a lot more
sense. (And to that end, review comments about what could be organized
better or needs more documentation are very much in order, even for code
that is mostly unchanged.)
This does introduce one bug, sort of, which is that rather than
generating broken code for list-of-interface fields, we generate no code
at all. (A TODO in unmarshal.go describes why.) I'll fix this when I
add support for those fields. (It's all behind the AllowBrokenFeatures
flag, anyway.) Otherwise, the only changes to generated code are that a
few methods are ordered differently, because we now generate the
implements-interface methods with the interface, rather than the
implementations, as it's much simpler that way. (In GraphQL, unlike Go,
we know the list of all possible implementations of each interface, so
this is possible.)
## Test plan:
golangci-lint run ./... && go test ./...
Author: benjaminjkraft
Reviewers: dnerdy, benjaminjkraft, aberkan, csilvers, MiguelCastillo
Required Reviewers:
Approved by: dnerdy
Checks: ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/51
## Test plan
read it
Author: benjaminjkraft
Reviewers: aberkan, dnerdy, MiguelCastillo
Required Reviewers:
Approved by: aberkan
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/58
## Summary:
As Mark pointed out in a review where I realized that the *example* had
a bogus config key (which I hadn't noticed earlier because it was just
using the default value), we should probably do a strict-unmarshal;
there's no reason you should have random extra keys in your config and
if you do it's probably a mistake. (Or maybe you're running a too-old
version of genqlient for your codebase, which you probably also want to
know.) Now we do.
## Test plan:
- `make check`
- in webapp, `go mod edit -replace github.com/Khan/genqlient=../genqlient`
then `make genqlient` produces no diffs (except go.mod/go.sum).
Author: benjaminjkraft
Reviewers: dnerdy, aberkan, csilvers, MiguelCastillo
Required Reviewers:
Approved by: dnerdy
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/55
## Summary:
We have lots of tests covering codegen, but not a lot that actually run
the code. For things where all we do is generate types, that's (mostly)
fine (especially now that we actually build the code), but as we
generate more nontrivial non-type code we need to actually run it.
So I wrote some integration tests that spin up a little gqlgen
server, and make calls to it; we can add more over time especially as
the JSON marshalling logic gets complex (to support fragments).
They're more work to write than the snapshot tests, but of course they
can test a lot more.
In addition to gqlgen, I pulled in testify assert/require, because I
really wanted to be able to use assert.Equal and such for these. I
didn't bother converting existing tests, although I assume they will
become useful elsewhere in time. Both gqlgen and testify are of course
only used in tests.
Fixes#21 and #24.
Issue: https://github.com/Khan/genqlient/issues/21
Issue: https://github.com/Khan/genqlient/issues/24
## Test plan:
make check
Author: benjaminjkraft
Reviewers: aberkan, dnerdy, benjaminjkraft, csilvers, MiguelCastillo
Required Reviewers:
Approved by: aberkan, dnerdy
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/50
## Summary:
It would be nice to have some linting beyond `go vet`! Now we do. I
started by copying the config from Khan/webapp. I did remove a couple
of staticcheck checks that I didn't feel were useful. (Note also that
exportloopref is the replacement for scopelint in newer golangci-lint.)
Included are all the needed lint fixes; most are stylistic but the
changes in the example are a (minor) bugfix.
Fixes#22.
Issue: https://github.com/Khan/genqlient/issues/22
## Test plan:
make check
Author: benjaminjkraft
Reviewers: aberkan, dnerdy, benjaminjkraft, csilvers, MiguelCastillo
Required Reviewers:
Approved by: aberkan, dnerdy
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/49
The design doc talks about the zero value for the `string` and `*string` types and then references `0` and `null`. This PR changes these values to `""` and `nil`.
Craig pointed out this is a bit confusing when you don't have all the
fields. Now we say so, but still include the type's description in case
it's useful.
Fixes#37.
