Skip to content
This repository has been archived by the owner on Apr 19, 2021. It is now read-only.

Adding Link to Node website , node and yarn installation instructions #325

Merged
merged 15 commits into from Mar 5, 2019
Merged

Adding Link to Node website , node and yarn installation instructions #325

merged 15 commits into from Mar 5, 2019

Conversation

a-deeb
Copy link
Contributor

@a-deeb a-deeb commented Mar 1, 2019

No description provided.

@a-deeb
Copy link
Contributor Author

a-deeb commented Mar 1, 2019

I added links to node and yarn install sites and added the install npm and yarn to the instructions

@aimenbatool aimenbatool mentioned this pull request Mar 1, 2019
Closed
@aimenbatool
Copy link
Contributor

aimenbatool commented Mar 1, 2019 •
edited

@a-deeb Please see the image below.

firefox health dashboard readme md at 9c38ba7bb2b1641dd17ee1d689a84328debc308e mozilla frontend infra firefox health dashboard

@klahnakoski can add more thoughts.

@a-deeb
Copy link
Contributor Author

a-deeb commented Mar 1, 2019

Hi, I just put the instructions under the headings, should they be in bullet form or should I remove that,
I also added a yarn troubleshooting section

@aimenbatool
Copy link
Contributor

Hi, I just put the instructions under the headings, should they be in bullet form or should I remove that,
I also added a yarn troubleshooting section

This looks good to me. @klahnakoski can add more into it.

@a-deeb
Copy link
Contributor Author

a-deeb commented Mar 1, 2019

yes, @aimenbatool sounds good!

klahnakoski
klahnakoski suggested changes Mar 3, 2019
View reviewed changes
Copy link
Contributor

@klahnakoski klahnakoski 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 these improvements so far.

Also, add a "testing" subsection (yarn test).

README.md Outdated Show resolved Hide resolved
README.md Outdated

## [Node](https://nodejs.org/en/)

* ## Install node:
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the star (*), just bold this line, or just remove it because we are in the Developing->Prerequisites-> Node section already

**Install node**

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed install node

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
## [Yarn](https://www.npmjs.com/package/yarn)


* ## Install yarn:
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line

README.md Outdated Show resolved Hide resolved
@a-deeb
Copy link
Contributor Author

a-deeb commented Mar 3, 2019

@klahnakoski Thanks for letting me help out

@klahnakoski klahnakoski merged commit a00b5a9 into mozilla-frontend-infra:master Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@klahnakoski klahnakoski klahnakoski requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@a-deeb @aimenbatool @klahnakoski