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

Feature/whats new #19

Open
wants to merge 4 commits into
base: feature/whatsNew
Choose a base branch
from

Conversation

jong-int
Copy link
Contributor

James' updates

Copy link
Contributor

@bcole bcole left a comment

Choose a reason for hiding this comment

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

Left a few minor comments. Also - can you raise this PR into gh-pages branch instead? feature/whatsNew was a feature we started but never finalized + never merged (it's not ready to merge).

You can create your own feature branch from gh-pages (call it say, feature/jamesChanges, or whatever), and merge into gh-pages. Thanks!

layout: default
title: Best Practices
nav_order: 4
parent: GraphQL Concepts
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to clarify, do you want this under GraphQL Concepts, or under FAQ?

If we want it under GraphQL Concepts, can we just move this file into the directory graphql-concepts/ (just for organization)? It's currently in the faq/ dir.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's move it under FAQ, makes most sense to me

Instead of specifying a list of field names we recommend creating a fragment as a best practice once and reuse it everywhere. This makes your queries modular and easy to understand. [Read more about fragments here](../../graphql-concepts/other-concepts/#fragments)


## Error Handling
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the intent was to remove Error Handling from this page (into its own page), right? We should remove it from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, was trying to move it, sorry!

parent: GraphQL Concepts
---

## Best Practices
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto as above, should remove this top section (Best Practices), and leave only Error Handling bottom section (assuming that was the intent here)

parent: GraphQL Concepts
---

## Best Practices
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto as above, should remove this top section (Best Practices), and leave only Error Handling bottom section (assuming that was the intent here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was trying to create two separate pages, one for errors and one for best practices - sorry I am still learning this

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