Skip to content

[EXPERIMENTAL] Run standard --fix #1371

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

Closed
wants to merge 1 commit into from
Closed

[EXPERIMENTAL] Run standard --fix #1371

wants to merge 1 commit into from

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Feb 10, 2017

This is a test PR, mainly to get the ball rolling and see what would happen if we simply applied standard formatting. There are lots of rules that will require manual rewriting.

In particular, the vast majority look like

convert.js:219:72: ':' should be placed at the beginning of the line.
index.js:34:3: Split initialized 'var' declarations into multiple statements.

It's probably a couple hour pass to go through and fix those, then a bit more work to figure out maybe a few more subtle issues.

See: #950 ping @etpinard

@etpinard
Copy link
Contributor

etpinard commented Feb 10, 2017

Hey, at least all the ; are gone!

@rreusser
Copy link
Contributor Author

rreusser commented Feb 10, 2017

Of note:eslint is the only phase of the tests this PR broke (which is not a foregone conclusion considering 408 files +41,098 / −42,357)

@rreusser
Copy link
Contributor Author

rreusser commented Feb 13, 2017

Common sentiment overheard: "OMG eslint is just horrible to have to run all the time. prettier solves this whole problem in a way that makes fundamentally so much more sense. So over eslint." Paraphrasing, of course. People seem very excited about dumping eslint for prettier though. In summary, eslint 'fixes' your code by tweaking it. Prettier parses the AST, dumps the original code, and reprints it with perfectly consistent and standardized formatting.

If I understand then, there aren't rules to violate. You write code. It reinterprets it and applies its own formatting. Seems nice.

@rreusser
Copy link
Contributor Author

Holding off until some large PRs have been merged.

@rreusser
Copy link
Contributor Author

FWIW:

screen shot 2017-02-14 at 14 23 25

@rreusser rreusser mentioned this pull request Feb 24, 2017
@rreusser rreusser mentioned this pull request Apr 26, 2017
@rreusser
Copy link
Contributor Author

rreusser commented May 1, 2017

Closing for now. Can revisit if changing the linting seems like the right move.

@rreusser rreusser closed this May 1, 2017
@etpinard etpinard deleted the standard branch May 16, 2017 19:23
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.

2 participants