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

Use yaml.UnmarshalStrict #55

Merged
merged 1 commit into from
Aug 23, 2021
Merged

Use yaml.UnmarshalStrict #55

merged 1 commit into from
Aug 23, 2021

Conversation

benjaminjkraft
Copy link
Collaborator

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).

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).

Reviewers: csilvers, adam, marksandstrom, miguel
Copy link
Contributor

@dnerdy dnerdy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! 🎉

@benjaminjkraft benjaminjkraft merged commit 6689d5a into main Aug 23, 2021
@benjaminjkraft benjaminjkraft deleted the benkraft.strict branch August 23, 2021 22:52
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 this pull request may close these issues.

2 participants