Skip to content

Suggestions for the migration docs #2718

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

Merged

Conversation

rkoron007
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Dec 9, 2022

👷 Deploy request for apollo-ios-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit d7cd66e

@rkoron007 rkoron007 self-assigned this Dec 13, 2022
@rkoron007 rkoron007 marked this pull request as ready for review December 13, 2022 19:49
#### Separating cache mutations from network operations
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason this entire this is using ">" is because it's a shared note that we are including in multiple places, but it's intended to be a callout. It's a section that gives some usage suggestions and design notes, but it's not directly related to the function of the articles in which it is included.

By not using the ">" it renders as it's own sub heading in the articles, rather than a callout box. And I don't think that looks right.

@rkoron007 @calvincestari @bignimbus Opinions on this? Here is a link to where it is rendered so you can see what it looks like.

https://www.apollographql.com/docs/ios/caching/cache-transactions/#separating-cache-mutations-from-network-operations

Copy link
Member

Choose a reason for hiding this comment

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

I think these notes are important enough to be rendered differently. So if there isn't another way to have them distinct from the text above I think using > is what we'll have to use.

Copy link
Member

Choose a reason for hiding this comment

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

@trevorblades one more for your input please.

> This is an important decision that must be made prior to configuring code generation. To determine the right option for your project, check our our [project configuration documentation](/project-configuration).
- `${MySchemaName}` provides a name for the namespace of your generated schema files.
- `${ModuleType}` configures how your generated schema types are included in your project.
> This is a crucial decision to make **before configuring code generation**. To determine the right option for your project, see [Project Configuration](../project-configuration).
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this relative path going to work correctly from all places? This is a shared mdx component, and it's used from pages at different paths. Not sure if the link gets resolved relative to the path of the shared component, or the path of the page it is included in? @trevorblades would probably know.

If relative won't work here, the correct solution I think is [Project Configuration](ios/project-configuration).

But that isn't future proof once this isn't the latest docs version anymore haha.

Copy link
Member

Choose a reason for hiding this comment

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

I believe it is relative to the page that includes it. @trevorblades can you confirm that for us please?

Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

Thanks for the editing @rkoron007, this looks great! I've added my comments where appropriate.

#### Separating cache mutations from network operations
Copy link
Member

Choose a reason for hiding this comment

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

I think these notes are important enough to be rendered differently. So if there isn't another way to have them distinct from the text above I think using > is what we'll have to use.

calvincestari and others added 2 commits December 16, 2022 16:30
Co-authored-by: Anthony Miller <anthonymdev@gmail.com>
@calvincestari
Copy link
Member

Ignore the CI failure in Codegen CLI Integration Tests, this PR hasn't been rebased to include #2715.

To begin your migration to Apollo iOS 1.0 you'll need to:
To migrate to Apollo iOS 1.0, you'll do the following:
1. [Update your Apollo iOS dependency](#1-update-to-apollo-ios-10)
2. [Setup code generation](#2-setup-code-generation)
Copy link
Contributor

Choose a reason for hiding this comment

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

changing the title means this link needs to change to `#2-set-up-code-generation

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@AnthonyMDev AnthonyMDev merged commit c2405c3 into apollographql:main Dec 19, 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.

None yet

3 participants