Skip to content

fix links and newlines in getting started#165

Merged
mhdawson merged 1 commit into
nodejs:masterfrom
RaisinTen:fix-links
Nov 27, 2020
Merged

fix links and newlines in getting started#165
mhdawson merged 1 commit into
nodejs:masterfrom
RaisinTen:fix-links

Conversation

@RaisinTen
Copy link
Copy Markdown
Member

@RaisinTen RaisinTen commented Nov 16, 2020

I was going through the docs and noticed that the links didn't work. So, I fixed them and the newlines. 🙂

@mhw0
Copy link
Copy Markdown

mhw0 commented Nov 21, 2020

Yes, some links don't work. I think this is because of the slug in the gatsby-node.js file.

@mhdawson
Copy link
Copy Markdown
Member

mhdawson commented Nov 23, 2020

@jschlight could you take a look?

@gengjiawen gengjiawen requested a review from jschlight November 24, 2020 03:29
jschlight
jschlight requested changes Nov 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jschlight jschlight left a comment

Choose a reason for hiding this comment

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

Thank you for reporting this issue and submitting a PR! In testing your PR, I've determined the correct form for the links is [prerequisites](prerequisites), [tools](tools), and [A first project](first). If you make these changes and resubmit I'll be happy to look at it again.

There is information at this link that explains how to build and test the site on your development system:

https://github.com/nodejs/node-addon-examples/blob/master/website/README.md#contributing

@RaisinTen
Copy link
Copy Markdown
Member Author

RaisinTen commented Nov 25, 2020

Thanks for the review! Actually I can't build gatsby on my IA-32 linux because compiling sharp produces the error discussed here: lovell/sharp#1882
I made the changes. :)

@jschlight jschlight self-requested a review November 25, 2020 19:22
jschlight
jschlight approved these changes Nov 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jschlight jschlight left a comment

Choose a reason for hiding this comment

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

LGTM with the changes.

mhdawson
mhdawson approved these changes Nov 27, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson mhdawson merged commit 0c6d64b into nodejs:master Nov 27, 2020
@RaisinTen RaisinTen deleted the fix-links branch November 27, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhdawson mhdawson mhdawson approved these changes

@jschlight jschlight jschlight approved these changes

+1 more reviewer

@helio-frota helio-frota helio-frota approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@RaisinTen @mhw0 @mhdawson @helio-frota @jschlight