Skip to content

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.

"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.


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.

7 participants