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

Set up lint and test #1

Merged
merged 9 commits into from Nov 9, 2017
Merged

Set up lint and test #1

merged 9 commits into from Nov 9, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Nov 8, 2017

Not sure what we want here, so I just threw together something that works.

  • Do we want flow?
  • Do we want babel?

Other:

  • Do we want pre-commit hooks for formatting?
  • Do we want Travis or Circle?
  • Do we want to support node 4?

@orta
Copy link
Member

orta commented Nov 8, 2017

As it's not an externally used lib flow isn't too important to me, and without flow then it doesn't need babel 👍

Do we want pre-commit hooks for formatting?

pre-commit with Prettier is a big 👍 for me

Travis vs Circle

Travis is used on some of the others?

Node 4

I don't have strong opinions - it's still actively supported by node though

@SimenB
Copy link
Member Author

SimenB commented Nov 8, 2017

Ok, made those changes.

Tests are failing on node 4 with a really useless error.

One issue with not dropping node 4 is that we won't be able to upgrade jest. Not a huge issue, I guess 🙂

EDIT: Stupid error. Actually running it on node made it obvious, not sure why the error thrown by jest (via new vm.Script()) is so useless.

/Users/simbekkh/repos/eslint-plugin-jest/ugh.js:3
const { RuleTester } = require('eslint');
      ^

SyntaxError: Unexpected token {
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:373:25)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:140:18)
    at node.js:1043:3

yarn: true
branches:
only:
- master
Copy link
Member

Choose a reason for hiding this comment

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

geez, I love Travis config being so simple compared to Circle 2

},
"prettier": {
"singleQuote": true,
"trailingComma": "all"
Copy link
Member

Choose a reason for hiding this comment

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

add "bracketSpacing": false maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

do you prefer it? Seems like a fb-ism. Happy to add, of course, I don't really care as long as it's formatted for me :D

Copy link
Member

Choose a reason for hiding this comment

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

I have no preference, just that the diff would be smaller :D. We can scratch that

@SimenB
Copy link
Member Author

SimenB commented Nov 9, 2017

#yolo

@SimenB SimenB merged commit 8cb5e56 into master Nov 9, 2017
@SimenB SimenB deleted the setup branch November 9, 2017 07:24
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.

None yet

3 participants