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

Improve reading of config file #1

Merged
merged 1 commit into from
Aug 5, 2017
Merged

Improve reading of config file #1

merged 1 commit into from
Aug 5, 2017

Conversation

voxpelli
Copy link
Contributor

As part of refactoring a GraphQL server I wanted to focus on getting the GraphQL Schema into the best shape possible and what better tool to help in that, and to ensure that it sticks that way, than a linter? So I was very happy to find that someone had started on making a linter for GraphQL Schemas, very handy!

I did however run across an error with the CLI command and tracked it down to an erroneous expectation of the return data from fs.readFileSync(). When an encoding has been specified, as it has in this case, then it returns a string: https://nodejs.org/api/fs.html#fs_fs_readfilesync_path_options

Due to this wrong expectation the defined config file wasn't used but instead a JSON parse error was given whenever a config file was defined.

As an additional fix I reworded the error message to actually note the reason to why it will run with the default config + removed a needless assign to a json const. I hope that's okay and not stretching it too far for a single PR.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.667% when pulling b425ac0 on voxpelli:patch-1 into 33c8263 on happylinks:master.

@happylinks happylinks merged commit 89681a3 into happylinks:master Aug 5, 2017
@happylinks
Copy link
Owner

Sorry this took so long! I didn't see the PR. Thanks for the changes!
Are you still using the linter? Curious to hear your thoughts about it :)

@voxpelli voxpelli deleted the patch-1 branch August 7, 2017 08:27
@voxpelli
Copy link
Contributor Author

voxpelli commented Aug 7, 2017

Thanks for merging @happylinks!

I'm still using it. I've made it part of the tests for our internal GraphQL server at @Sydsvenskan and I'm happy so far 👍

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.

3 participants