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

chore(eslint): upgrade to eslint@4 #871

Merged
merged 1 commit into from
Jun 23, 2017

Conversation

mastilver
Copy link
Contributor

close #870

@mastilver mastilver force-pushed the upgrade-to-eslint-4 branch 2 times, most recently from a93d90e to 3f8bfdf Compare June 12, 2017 13:31
@coveralls
Copy link

coveralls commented Jun 12, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling 3f8bfdf on mastilver:upgrade-to-eslint-4 into 3c46d30 on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 12, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling 3f8bfdf on mastilver:upgrade-to-eslint-4 into 3c46d30 on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 12, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling 3f8bfdf on mastilver:upgrade-to-eslint-4 into 3c46d30 on benmosher:master.

Copy link

@corbinu corbinu left a comment

Choose a reason for hiding this comment

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

peerDependencies also needs to be updated to allow for eslint 4

@mastilver mastilver force-pushed the upgrade-to-eslint-4 branch 2 times, most recently from f5dbba8 to 2181590 Compare June 12, 2017 15:27
@coveralls
Copy link

coveralls commented Jun 12, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling f5dbba8 on mastilver:upgrade-to-eslint-4 into 3c46d30 on benmosher:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.157% when pulling f5dbba8 on mastilver:upgrade-to-eslint-4 into 3c46d30 on benmosher:master.

@mastilver
Copy link
Contributor Author

The failing tests are also failing on master, I will try to find what's going on

I would like to get some help to apply the same logic for appveyor as I did for travis! :)

@ShayDavidson
Copy link

Hey, any update on this? Thanks!

@Arcanemagus
Copy link
Contributor

@mastilver For AppVeyor you will need to put each of the 6 combinations as individual groups in the environment matrix, so something like this:

environment:
  matrix:
    - nodejs_version: "7"
      ESLINT_VERSION: "4"

    - nodejs_version: "7"
      ESLINT_VERSION: "3"

    - nodejs_version: "7"
      ESLINT_VERSION: "2"

    - nodejs_version: "6"
      ESLINT_VERSION: "4"

    - nodejs_version: "6"
      ESLINT_VERSION: "3"

    - nodejs_version: "6"
      ESLINT_VERSION: "2"

Note also that it seems npm v3 is being forcibly installed for some reason, possibly an artifact from when Node.js v0.10 was still supported. As this is a downgrade for both of those versions of Node.js this should probably be removed unless it is still testing something relevant.

@ljharb
Copy link
Member

ljharb commented Jun 15, 2017

npm 3 is absolutely still supported, as is npm 2, because that's what node 4 and 6 ship with.

That said, we shouldn't be forcibly installing an older version of npm.

@alexander-akait
Copy link

/cc @mastilver friendly ping

@mastilver
Copy link
Contributor Author

@evilebottnawi Yeah, I know... :)

I need to allocate more time for it, I will continue tonight or Sunday
In the meantime if someone want to take over feel free or ping me and I'll add you to the fork

@coveralls
Copy link

coveralls commented Jun 17, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling 72472fd on mastilver:upgrade-to-eslint-4 into 3f9e4bf on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 17, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling eec46df on mastilver:upgrade-to-eslint-4 into 3f9e4bf on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 17, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling eec46df on mastilver:upgrade-to-eslint-4 into 3f9e4bf on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 17, 2017

Coverage Status

Coverage remained the same at 95.157% when pulling ca8c28b on mastilver:upgrade-to-eslint-4 into 3f9e4bf on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling c58fad7 on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@benmosher
Copy link
Member

benmosher commented Jun 22, 2017

@mastilver

remove eslint matrix for appveyor and just keep the node matrix

sounds good to me 👍🏻

@Arcanemagus
Copy link
Contributor

Looks like the failing Travis CI build is on a test that normally takes a while to run, as that specific job was the longest running is that an intermittent failure on the slower OS X hosts?

@mastilver mastilver force-pushed the upgrade-to-eslint-4 branch 2 times, most recently from 528fc71 to 4c52e1a Compare June 22, 2017 18:01
@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling 4c52e1a on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling 4c52e1a on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@mastilver
Copy link
Contributor Author

@benmosher I think it would be handy to enable Auto cancel pull request builds on travis

@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling a8ca5bb on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@coveralls
Copy link

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 95.963% when pulling a8ca5bb on mastilver:upgrade-to-eslint-4 into 0263be4 on benmosher:master.

@benmosher
Copy link
Member

Ah, neat. I didn't even know that was an option, thanks! And great work! 🎉🙏🏻🎉

@benmosher benmosher merged commit 7f055ec into import-js:master Jun 23, 2017
@lukeapage
Copy link
Contributor

@benmosher please can we have a new release to unblock using eslint 4?

@ljharb
Copy link
Member

ljharb commented Jun 23, 2017

@lukeapage #870 (comment) please be patient. Using eslint 4 isn't urgent :-)

@lukeapage
Copy link
Contributor

Sorry should have checked the issue, I thought it had just been forgotten.

@mastilver mastilver deleted the upgrade-to-eslint-4 branch June 23, 2017 09:25
@benmosher
Copy link
Member

published 👍🏻

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

Successfully merging this pull request may close these issues.

ESlint v4
9 participants