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.

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

Make it consistent #984

Merged
merged 1 commit into from
Jun 14, 2017
Merged

Make it consistent #984

merged 1 commit into from
Jun 14, 2017

Conversation

joaojeronimo
Copy link
Contributor

No description provided.

Copy link
Collaborator

@nfcampos nfcampos left a comment

Choose a reason for hiding this comment

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

yeah why not

@nfcampos nfcampos merged commit 0d1936a into enzymejs:master Jun 14, 2017
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Actually these should all have semicolons, per airbnb's style guide. Please make a followup PR to add them.

@nfcampos
Copy link
Collaborator

@ljharb you might be interested to know that this is the result of running all code inside our markdown through eslint (after disabling some rules that arguably don't really apply to code snippets)
screen shot 2017-06-15 at 22 33 56

@nfcampos
Copy link
Collaborator

so do we really want to fix these 6 semicolons but ignore the other +900 deviations from the style guide?

@ljharb
Copy link
Member

ljharb commented Jun 16, 2017

Let's fix one rule at a time, but yes, they should all be fixed.

It'd be great to add this check to the tests, so that incorrect code can be kept out of examples in the first place.

@nfcampos
Copy link
Collaborator

@ljharb its trivial to add this check, the problem is we can't add this check without first fixing all existing errors

@ljharb
Copy link
Member

ljharb commented Jun 16, 2017

If you can point me to a branch I can take a look at that.

@nfcampos
Copy link
Collaborator

here it is https://github.com/airbnb/enzyme/compare/eslint-markdown?expand=1

I've disabled some rules that arguably don't make too much sense for snippets (bringing the errors down to ~500)

@nfcampos
Copy link
Collaborator

one thing to bear in mind is that eslint-plugin-markdown does not support eslint --fix so any fixes will either be manual or require some custom code

@ljharb
Copy link
Member

ljharb commented Jun 16, 2017

Thanks, I'll do it manually this weekend; I can fix 500 eslint errors manually with my eyes closed :-p

@nfcampos
Copy link
Collaborator

eheh good luck :)

@ljharb ljharb mentioned this pull request Jun 17, 2017
@ljharb
Copy link
Member

ljharb commented Jun 17, 2017

Done: #988

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

Successfully merging this pull request may close these issues.

3 participants