-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Migrate to CircleCI2.0 and Add AppVeyor for master-only branch #11605
Conversation
name: Install Yarn | ||
command: | | ||
if [[ ! -e ~/.yarn/bin/yarn || $(yarn --version) != "${YARN_VERSION}" ]]; then | ||
curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version $YARN_VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to pin the version? docker/node comes with at least yarn 1.2.1 already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess circleci/node:8
uses yarn@1.3.1
.
build: | ||
|
||
docker: | ||
- image: circleci/node:8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worth using 8-alpine
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using circleci
docker images (https://hub.docker.com/r/circleci/node/tags/)
Unfortunately none existent image with Alpine. But this is nice question: Move foward with 8-alpine
(https://hub.docker.com/_/node/) ?
.circleci/config.yml
Outdated
command: ./scripts/circleci/test_entry_point.sh | ||
|
||
- save_cache: | ||
key: node-modules-{{ checksum "yarn.lock" }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably worth prefixing/appending -v1
to the key or something so it can be neatly manually invalidated if required
lsorry for all the notes :) i realize that most of those things are a straight port from the v1 config, but worth thinking about while the pieces are being moved |
No problem at all @jquense.
I totally agree. |
I created an AppVeyor project. What's the next step? |
@gaearon note that AppVeyor it's running only for |
I think that's it. |
Note: I've removed "Upload build" step on CircleCi 2.0 config in favour of #11666 |
# - npm ls --depth=0 | ||
cache_directories: | ||
- ~/.yarn | ||
- ~/.yarn-cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these irrelevant now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Yup @gaearon. I made some changes based on official docs about Caching for 2.0 and reading yarn package CircleCI configuration code. I'll sent a different PR for add AppVeyor badge on BTW: Note the warning on 1.0 build (e.g: https://circleci.com/gh/facebook/react/7853#tests/containers/3): |
Looks good to me. Thanks. |
I so missed the notification for this, sorry. FWIW, the CircleCI config looks good. I did notice it's missing the opportunity to store test results and thus the ability to graph testing data overtime. We can discuss in a new Issue if anyone is interested, just let me know. |
Ref: #11595
Result of AppVeyor on my fork: https://ci.appveyor.com/project/raphamorim/react/build/1.0.30
Result of CircleCI 2.0 on my fork: https://circleci.com/gh/raphamorim/react/21
cc// @gaearon, @FelicianoTech