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.

Sign up for GitHub

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

Jump to bottom

Update README.md #83

Merged
merged 1 commit into from
Oct 9, 2015
Merged

Update README.md #83

merged 1 commit into from
Oct 9, 2015

Conversation

bnjmnt4n
Copy link
Contributor

  • Use smart quotes
  • Update some choose blocks
  • Shift location of Travis image

@bnjmnt4n
Update README.md
9a51eff 
 * Use smart quotes
 * Update some choose blocks
 * Shift location of Travis image
@bnjmnt4n
Copy link
Contributor Author

Ping? ;)

@aseemk
Copy link
Member

aseemk commented Jan 17, 2015

Sorry about that, @D10! I missed this over the holidays. Thanks for the ping. =)

@gasi would love you for the change in quotes. =D

I will review and would be happy to merge after any changes. Thank you for the PR!

aseemk
aseemk reviewed Jan 17, 2015
View reviewed changes
README.md
```
npm install json5
```sh
$ npm install json5
Copy link
Member

Choose a reason for hiding this comment

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

The lack of $ in these bash snippets was actually intentional: it lets you copy-paste directly.

(There have been some fun blog posts written on this topic, and I just spent an unreasonable amount of time trying to find one, but couldn't. Hard to search for them...)

So would you mind reverting the additions of $ in these snippets? Thanks!

Copy link

Choose a reason for hiding this comment

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

@jordanbtucker What about this comment? 👀

Copy link
Member

Choose a reason for hiding this comment

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

@gasi I fixed them in post rather than wait for the submitter to fix. See cb2613e.

Copy link

Choose a reason for hiding this comment

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

👍

@aseemk
Copy link
Member

aseemk commented Jan 17, 2015

LGTM otherwise, thank you @D10!

@gasi
Copy link

gasi commented Jan 19, 2015

👍

@jordanbtucker jordanbtucker merged commit 9a51eff into json5:master Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bnjmnt4n @aseemk @gasi @jordanbtucker