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

Reject a glob that matches no files #173

Merged
merged 1 commit into from
Feb 10, 2022
Merged

Conversation

benjaminjkraft
Copy link
Collaborator

The most important case here is if your glob isn't even a glob, it's
just a filename. But even if it was a glob, it's probably a mistake;
you'll probably end up with a confusing error due to an empty schema, or
a slightly less confusing error due to not having any operations.
Instead, let's just say outright that your glob didn't match any files.

Fixes #146.

Test plan:
This was a bit annoying to test via snapshot, so I just tested it
manually by modifying the example to use a glob that didn't match any
files, first for the schema then for the operations, and got errors like

bogus*.graphql did not match any files
exit status 1
example/main.go:68: running "go": exit status 1

I have:

  • Written a clear PR title and description (above)
  • Signed the Khan Academy CLA
  • Added tests covering my changes, if applicable (see above)
  • Included a link to the issue fixed, if applicable
  • Included documentation, for new features (n/a)
  • Added an entry to the changelog (n/a, fixing a bug not in a release)

The most important case here is if your glob isn't even a glob, it's
just a filename.  But even if it was a glob, it's probably a mistake;
you'll probably end up with a confusing error due to an empty schema, or
a slightly less confusing error due to not having any operations.
Instead, let's just say outright that your glob didn't match any files.

Test plan:
This was a bit annoying to test via snapshot, so I just tested it
manually by modifying the example to use a glob that didn't match any
files, first for the schema then for the operations, and got errors like
```
bogus*.graphql did not match any files
exit status 1
example/main.go:68: running "go": exit status 1
```
Copy link
Member

@StevenACoffman StevenACoffman left a comment

Choose a reason for hiding this comment

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

Hmmm, yeah tricky to test, so fine to just do it without coverage.

Copy link
Member

@csilvers csilvers left a comment

Choose a reason for hiding this comment

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

Seems good to me!

@benjaminjkraft benjaminjkraft mentioned this pull request Feb 10, 2022
6 tasks
@benjaminjkraft benjaminjkraft merged commit 49a26af into main Feb 10, 2022
@benjaminjkraft benjaminjkraft deleted the benkraft.glob-matched-none branch February 10, 2022 00:55
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.

On main, there is no error if the schema file cannot be found
3 participants