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

Init Config Improvement #260

Merged
merged 3 commits into from
Aug 7, 2018
Merged

Init Config Improvement #260

merged 3 commits into from
Aug 7, 2018

Conversation

mathewbyrne
Copy link
Contributor

@mathewbyrne mathewbyrne commented Aug 6, 2018

Init will currently fail with an obscure error message if a config file is found. This change fixes a couple of issues with the codepath, and outputs a better error message if a config file is located:

init failed: a configuration file already exists at /Users/mathew/Projects/go/src/github.com/99designs/example/gqlgen.yml

Also fixed a minor issue outputting the server filename.

os.Exit(1)
}

if !os.IsNotExist(errors.Cause(err)) {
Copy link
Member

Choose a reason for hiding this comment

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

What if the above code found a config file but there's parsing errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is a parse error then config will be nil and it will exit with 1 right? I think that's the correct behaviour. What do you think should happen?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah cool yeah that's what I am thinking too so we will need to handle that edge case then because I don't think
!os.IsNotExist(errors.Cause(err)) will catch it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note the ! — I know it's a bit hard to read, but it's checking for the opposite case; basically any error except file not found.

Copy link
Member

Choose a reason for hiding this comment

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

hrmm okay I guess that makes sense.

@mathewbyrne mathewbyrne merged commit c57619e into master Aug 7, 2018
@porty porty deleted the init-improvements branch August 22, 2018 17:44
cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
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