-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Reorganize Server docset information architecture #3214
Conversation
bed2a49
to
7b36d20
Compare
4a80881
to
2201a98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍 Thanks for working on this! I've left feedback throughout, but I'm pretty happy with the direction.
docs/gatsby-config.js
Outdated
'defining-a-schema/directives', | ||
'defining-a-schema/creating-directives', | ||
], | ||
'Resolving Requests': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having a hard time with Resolving Requests, but I don't immediately have another concrete suggestion coming to mind.
Spitballing: Configuration execution? Configuring the server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to "Serving requests" which I'm not sure would feel like an improvement to you? At least it feels like a reasonable category to me for "read this stuff to learn how to actually do server stuff"
docs/source/getting-started.md
Outdated
) | ||
``` | ||
|
||
Hapi follows the same pattern with `apollo-server-express` replaced with `apollo-server-hapi` and `app` replaced with Hapi server. `applyMiddleware` registers plugins, so it should be called with `await`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't introduced by this PR (I don't think?) but, contextually, I find this note about Hapi a bit random unless it's trying to lay out an example of similar patterns being available for other frameworks. If that's what it's doing though, I think it falls way short since the patterns are often quite a bit different from framework to framework. I think the actual README.md
files within the individual framework's directories within packages/
actually do lay out the more accurate steps, but I'm not positive if they're entirely up to date.
Regardless, this whole pattern will go away with Apollo Server 3.x if #3184 goes through (which I suspect it will, one way or another).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will look into this tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified this hapi language in the new article middleware.md
that pulls that information out of the getting-started
07214f4
to
98156b3
Compare
c0223f5
to
36bb0e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is all incredible. Thank you, @StephenBarlow!
I left one point below, which I'm still hung up on but don't have a better suggestion for other than Features (... Configuring components?), but otherwise, I've reviewed all the edits and I think they're marvelous.
docs/gatsby-config.js
Outdated
], | ||
'Serving Requests': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section title and the slug for it is the only thing that I mildly struggle with. e.g. Serving errors, Serving file uploads, Serving data sources.
That's to say, I'm not sure that you actually serve any of those, but they are configurable components of a request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the title "Serving requests" either. What about "Fetching data"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @StephenBarlow! A couple minor changes, but otherwise I'm really happy with this new direction! 🎉
docs/gatsby-config.js
Outdated
], | ||
'Serving Requests': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the title "Serving requests" either. What about "Fetching data"?
b24c269
to
7313392
Compare
7313392
to
ae3f6b2
Compare
This is now ready for review! Happy to address any and all other feedback on this information architecture.