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

fix running without options #120

Merged
merged 1 commit into from
Jan 2, 2017
Merged

fix running without options #120

merged 1 commit into from
Jan 2, 2017

Conversation

flying-sheep
Copy link
Contributor

@flying-sheep flying-sheep commented Dec 5, 2016 •
edited
Loading

check out the single change by disabling whitespace diffing

@jsf-clabot
Copy link

jsf-clabot commented Dec 5, 2016 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@eslintbot
Copy link

Thanks for the pull request, @flying-sheep! I took a look to make sure it's ready for merging and found some changes are needed:

  • The commit summary needs to begin with a tag (such as Fix: or Update:). Please check out our guide for how to properly format your commit summary and update it on this pull request.

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@eslintbot
Copy link

Thanks for the pull request, @flying-sheep! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

JamesHenry
JamesHenry suggested changes Dec 13, 2016
View reviewed changes
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this @flying-sheep! Just a quick update and then we'll get this merged.

tests/lib/parse.js
describe("basic functionality", function() {

it("should parse an empty string", function() {
parser.parse("");
Copy link
Member

Choose a reason for hiding this comment

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

Please add simple assertions for the resulting ASTs - they will be small enough to keep inline. I would prefer to be explicit rather than the successful test simply be the absence of an exception

Copy link
Contributor Author

@flying-sheep flying-sheep Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

done!

and i fixed your .eslintrc

@eslintbot
Copy link

LGTM

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Dec 13, 2016 •
edited
Loading

travis failure is #128

@flying-sheep
Copy link
Contributor Author

@JamesHenry could you approve please?

@JamesHenry
Copy link
Member

As soon as #131 gets merged you can rebase this and we'll get it merged! Thanks a lot for your patience

@JamesHenry
Copy link
Member

It's in! Please rebase the PR and we'll see where we're at 😄

@eslintbot
Copy link

LGTM

@flying-sheep
Copy link
Contributor Author

@JamesHenry done!

@JamesHenry JamesHenry merged commit ff283aa into eslint:master Jan 2, 2017
@JamesHenry
Copy link
Member

Thanks!

@flying-sheep flying-sheep deleted the patch-1 branch January 9, 2017 10:28
@flying-sheep
Copy link
Contributor Author

great!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JamesHenry JamesHenry JamesHenry approved these changes

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
@flying-sheep @jsf-clabot @eslintbot @JamesHenry