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

fix(docs): Begin a "Troubleshooting" section #142

Merged
merged 4 commits into from
Sep 3, 2018
Merged

fix(docs): Begin a "Troubleshooting" section #142

merged 4 commits into from
Sep 3, 2018

Conversation

Corim
Copy link
Contributor

@Corim Corim commented Aug 31, 2018

Troubleshooting issue added based on #97

@coveralls
Copy link

coveralls commented Aug 31, 2018

Pull Request Test Coverage Report for Build 581

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.762%

Totals Coverage Status
Change from base Build 576: 0.0%
Covered Lines: 217
Relevant Lines: 219

💛 - Coveralls

@maticzav
Copy link
Owner

maticzav commented Sep 1, 2018

Hey @Corim 👋!

What a great idea of adding the Troubleshooting section. I love your addition. The only thing I would change, if you don't mind, is the last sentence; I wouldn't say the response becomes invalid as I believe this might paint the wrong picture but rather that GraphQL sticks to its definition.

I am not sure whether we want to keep this as short as possible and only present the solution or genuinely acknowledge what happens behind the scenes. Maybe the following comment can better portray what I have in mind;

#126 (comment)

Tell me what you think! 🙂

@Corim
Copy link
Contributor Author

Corim commented Sep 1, 2018

I just wanted to start by saying I think you’ve done a great job with this project!

I do agree that the last sentence should be changed.

In regards to the solution length, I think a clear, concise answer helps keep out the noise, but I also think your explanation in #126 is tremendously helpful. I’m not sure how taboo it is, but a temporary solution - resulting in the fastest buildup of a robust troubleshooting section - might be to link to the issues (in this case #126 and #97) that already have explanations and examples. These links could be changed in the future to reference a blog post or document containing a lengthy and detailed answer.

I wouldn’t say this is the “correct” method, but linking to the issues that already have written explanations would definitely be an effective method to utilize work you’ve already done (until more time can be put into writing up comprehensive answers).

Going forward, a few other troubleshooting issues that come to mind relate to caching and "trying to use the permissions system as a filter" (which obviously doesn’t work, but it did take me some time to realize why)

As a newbie, I’ve made all the now-obvious mistakes. I will commit these changes, let me know what you think.

Thanks for all the work you've done!

@maticzav
Copy link
Owner

maticzav commented Sep 3, 2018

@Corim, this is such an amazing PR! I’ll merge it right away; thanks for addressing suggestions from above.

I hope you add all the issues you can think of, let me know if you need any help!

Thanks again 🎉

@maticzav maticzav merged commit 960b5ab into maticzav:master Sep 3, 2018
@maticzav
Copy link
Owner

maticzav commented Sep 3, 2018

🎉 This PR is included in version 3.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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