chore: move to circleci by shellscape · Pull Request #2390 · rollup/rollup

shellscape · 2018-08-08T17:07:53Z

This PR contains:

Breaking Changes?

yes
no

If yes, please describe the breakage.

Please Describe Your Changes

This PR proposes to move from Travis CI to Circle CI. Benefits will include:

Faster test run times
Better resource allocation
Use of custom containers for control of the environment
Linting, Test, and Coverage workflows to improve CI for Pull Requests

TrySound · 2018-08-08T17:28:53Z

-  }
+	"name": "rollup",
+	"version": "0.64.1",
+	"description": "Next-generation ES module bundler",


Could you not do this prettier change?

WIP :) this is not yet ready for review. Please hold off until that tag is removed.

@TrySound it's ready 🎉

shellscape · 2018-08-09T02:46:36Z

This PR is ready for review. A few notes:

CircleCI's Github notification on this PR has been muted using a setting that prevents it from showing on other PRs until this is ready to move forward.
Node v4 has been removed as it is out of all stages of LTS maintenance, and unsupported by Node.
Average run time on Circle under the proposed configuration is between 5 and 6 minutes. That compares to an average 15 minute runtime on Travis (loose figures from a quick glance through logs)
This configuration also runs linting as a CI job and will mean that PRs must pass linting to pass CI.
This configuration adds a security audit using npm audit
Dockerfiles for the containers used by this configuration can be found here: https://github.com/rollup/containers

The following tasks need to be performed after this PR has been approved, but before merging:

CircleCI notification setting needs to be enabled
TravisCI webhook needs to be removed in the TravisCI interface (and not through Github. Otherwise we'll get some build wonkiness in PRs for a little bit. Travis' webhook tends to be quite sticky)

guybedford · 2018-08-09T17:34:25Z

I'm just wondering what would we need to do to get the CircleCI check to display in the PR here?

shellscape · 2018-08-09T17:42:33Z

@guybedford see my previous comment. it's a global setting in CircleCI. If I turn it on here, it tends to display false positives on other PRs. To view the last build of this branch: https://circleci.com/gh/rollup/rollup/tree/chore%2Ftravis-to-circle

guybedford · 2018-08-11T10:01:31Z

The times do look pretty good there I must say. Perhaps this will even help with our watcher file descriptor flake issues :)

guybedford · 2018-08-11T10:01:57Z

 environment:
  matrix:
    # node.js
-    - nodejs_version: 4


We should keep Node 4 for now I think.

I think you may remove node4 support with 0.65 release

Node 4 has been out of active and maintenance LTS since April, and has passed end of live and is unsupported. (https://github.com/nodejs/Release#end-of-life-releases) This is a prudent change.

Fair enough. Given that I agree with @shellscape to remove it for 0.65. But we should have a discussion before we actually change the TypeScript output target to match that in the future as this would have some hidden implications.

Debian stable packages 4.8.2, and since sindresorhus/pretty-ms@b5a583d, rollup doesn't run on nodejs 4 anymore.

guybedford

This has my approval, but @lukastaegert please check this one as well before merge.

lukastaegert

Great work. One final note from me: Currently, npm run lint uses the --fix options. While this is probably exactly what we want locally and in the commit hook, the check on CI should probably skip the fixing since it is not committing any changes after that.

Instead I would prefer if CI would run a different script, e.g. npm run lint:ci, that does not auto-correct so that we are actually sure only valid code is merged.

…lup into chore/travis-to-circle

shellscape · 2018-08-12T20:40:56Z

@lukastaegert requested changes made!

lukastaegert · 2018-08-13T05:44:07Z

+    "ci:lint": "npm run lint:nofix && npm run security",
    "ci:test": "npm run test && npm run build:bootstrap && npm run test:only",
    "lint": "tslint --project . --fix && eslint --fix test/test.js test/*/index.js test/utils.js test/**/_config.js",
+    "lint:nofix": "tslint --project . && eslint test/test.js test/*/index.js test/utils.js test/**/_config.js",


lukastaegert

Nice! From my side you can merge and switch CIs at any time

shellscape · 2018-08-13T12:22:42Z

@lukastaegert thanks! The last build for the merge succeeded (https://circleci.com/gh/rollup/rollup/99), so we're good to go.

* Refactorings from #331 * Update dependencies * Update dependencies

shellscape added 2 commits August 8, 2018 13:05

chore: move to circleci

4674dba

chore: add noop travis config

ad98d4a

TrySound reviewed Aug 8, 2018

View reviewed changes

shellscape added 4 commits August 8, 2018 22:24

chore: revert prettier, coverage fix attempt

b5f53a6

chore: add node v10, remove v4 in appveyor config

261374d

chore: rever test/hooks/index

5670b2a

chore: properly revert test/hooks/index, nuke travis config

7db4905

shellscape changed the title ~~(WIP) chore: move to circleci~~ chore: move to circleci Aug 9, 2018

shellscape requested review from Rich-Harris, guybedford and lukastaegert August 9, 2018 02:47

shellscape mentioned this pull request Aug 9, 2018

Fix code coverage reporting #227

Closed

guybedford reviewed Aug 11, 2018

View reviewed changes

guybedford approved these changes Aug 11, 2018

View reviewed changes

lukastaegert reviewed Aug 12, 2018

View reviewed changes

shellscape and others added 3 commits August 12, 2018 08:50

Merge branch 'master' into chore/travis-to-circle

f6f0f4f

chore: ci:lint should not --fix

9dbee3a

chore: Merge branch 'chore/travis-to-circle' of github.com:rollup/rol…

9f388f4

…lup into chore/travis-to-circle

fix: linting errors in hooks/watch tests

4d25dd9

lukastaegert reviewed Aug 13, 2018

View reviewed changes

lukastaegert approved these changes Aug 13, 2018

View reviewed changes

Merge branch 'master' into chore/travis-to-circle

91f9fbf

shellscape merged commit ed8693b into master Aug 13, 2018

shellscape deleted the chore/travis-to-circle branch August 13, 2018 12:24

adrianheine referenced this pull request in rollup/rollup-plugin-commonjs Oct 11, 2018

Refactorings from #331 (#346)

6c93eb3

* Refactorings from #331 * Update dependencies * Update dependencies

Uh oh!

Uh oh!

Conversation

shellscape commented Aug 8, 2018

Breaking Changes?

Please Describe Your Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shellscape commented Aug 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guybedford commented Aug 9, 2018

Uh oh!

shellscape commented Aug 9, 2018

Uh oh!

guybedford commented Aug 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guybedford left a comment

Choose a reason for hiding this comment

Uh oh!

lukastaegert left a comment

Choose a reason for hiding this comment

Uh oh!

shellscape commented Aug 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukastaegert left a comment

Choose a reason for hiding this comment

Uh oh!

shellscape commented Aug 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shellscape commented Aug 9, 2018 •

edited

Loading