Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

docs(migrating-to-v2): fix typos #7892

Merged
merged 2 commits into from
Sep 6, 2018
Merged

Conversation

macklinu
Copy link
Contributor

@macklinu macklinu commented Sep 5, 2018

This PR cleans up a few typos / examples in the migrating from v1 to v2 guide.

@@ -407,7 +407,7 @@ Now, to add state to a link, pass it via a `state` prop.
```jsx
const NewsFeed = () => (
<div>
<Link to="photos/123" state={{ fromNewsFeed: true }} />
<Link to="photos/123" state={{ fromFeed: true }} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The example was mixing fromNewsFeed and fromFeed, but this change makes it consistent.

image

@@ -843,7 +843,7 @@ In most cases you won't have to do anything to be v2 compatible, however there a

```json
"peerDependencies": {
"gatsby": ">=1"
"gatsby": ">=2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this should read 2 instead of 1 if I'm reading this section correctly, which is explaining how plugin authors can update for v2 support.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

If the plugin works for Gatsby v1 and v2 then >=1 is fine. A lot of plugins specify 1 as the supported version.

Maybe this docblock could be changed from json to diff and updated to show the change?

-  "gatsby": "1"
+  "gatsby": ">=1"

Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

Thanks @macklinu 👍

I've requested one small change, otherwise all good!

@@ -843,7 +843,7 @@ In most cases you won't have to do anything to be v2 compatible, however there a

```json
"peerDependencies": {
"gatsby": ">=1"
"gatsby": ">=2"
Copy link
Contributor

Choose a reason for hiding this comment

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

If the plugin works for Gatsby v1 and v2 then >=1 is fine. A lot of plugins specify 1 as the supported version.

Maybe this docblock could be changed from json to diff and updated to show the change?

-  "gatsby": "1"
+  "gatsby": ">=1"

@macklinu
Copy link
Contributor Author

macklinu commented Sep 5, 2018

@m-allanson thanks for the feedback! I've addressed your comment and pushed up.

m-allanson
m-allanson previously approved these changes Sep 5, 2018
@macklinu
Copy link
Contributor Author

macklinu commented Sep 5, 2018

@m-allanson I rebased the latest master into my branch and pushed up and didn't realize it would dismiss your review, my bad 😅

Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @macklinu 👍

@m-allanson m-allanson merged commit 015dddb into master Sep 6, 2018
@m-allanson m-allanson deleted the macklinu/v1-to-v2-migration-docs branch September 6, 2018 07:13
@m-allanson
Copy link
Contributor

m-allanson commented Sep 6, 2018

The dismissed reviews are a bit unexpected, I've been caught by that too! Generally I wouldn't worry about rebasing for pull requests unless something is broken. As the rebase changes the commit times it can make it harder to see what's changed between PR reviews. All good on this one though, thanks for handling these updates :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants