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

Add prettier #409

Merged
merged 2 commits into from
Apr 21, 2017
Merged

Add prettier #409

merged 2 commits into from
Apr 21, 2017

Conversation

danez
Copy link
Member

@danez danez commented Mar 6, 2017

Enables prettier

//cc @vjeux Not many drastic changes here, but in case you want to have a look

//cc @hzoo @yavorsky How should we handle eslint? I disabled one rule arrow-parens as it was interfering, should we extend eslint-config-prettier in our config?

@danez danez added the chore label Mar 6, 2017
@codecov
Copy link

codecov bot commented Mar 6, 2017

Codecov Report

Merging #409 into master will decrease coverage by 3.2%.
The diff coverage is 58.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
- Coverage   84.18%   80.97%   -3.21%     
==========================================
  Files           6        6              
  Lines         177      184       +7     
  Branches       41       49       +8     
==========================================
  Hits          149      149              
- Misses         12       19       +7     
  Partials       16       16
Impacted Files Coverage Δ
src/utils/read.js 75% <ø> (ø) ⬆️
src/resolve-rc.js 94.11% <ø> (ø) ⬆️
src/utils/exists.js 100% <100%> (ø) ⬆️
src/fs-cache.js 66.1% <25%> (-6.45%) ⬇️
src/index.js 86.04% <92.85%> (-1.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74ff2e6...9a44bbc. Read the comment docs.

package.json Outdated
"prettier --trailing-comma all --write ",
"git add"
],
"test/**/*.test.js": [
Copy link
Member

Choose a reason for hiding this comment

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

Should we also include test/helpers/createTestDirectory.js ?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do all of these or can we just run it on all .js files?

Copy link
Member

@yavorsky yavorsky Mar 9, 2017

Choose a reason for hiding this comment

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

@hzoo I think the only reason why we can't do this is that running on all .js files will affect fixtures

@yavorsky
Copy link
Member

yavorsky commented Mar 6, 2017

eslint-config-prettier is a good idea. I'm going to add it for babel/babel-preset-env#183.

But seems like adding

"arrow-parens": "off",
"indent": "off"

is enough to make babel-config prettier-friendly.

@@ -57,13 +56,12 @@ const write = function(filename, result, callback) {
const content = JSON.stringify(result);

return zlib.gzip(content, function(err, data) {
if (err) { return callback(err); }
if (err) return callback(err);
Copy link
Member

Choose a reason for hiding this comment

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

I like the braces but ok

Copy link

Choose a reason for hiding this comment

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

Note that this is not prettier doing. Prettier will keep the {} as they were inputted. But, if there are braces, the expression will be on its own line, if there are no braces, the expression is inline like it is here.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I didn't think so

@existentialism
Copy link
Member

It might be fine to just remove the few style rules we have in eslint-config-babel?

@danez danez changed the base branch from master to 7.0 April 21, 2017 19:37
@danez danez merged commit 2204871 into 7.0 Apr 21, 2017
@danez danez deleted the prettier branch April 21, 2017 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants