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

Migrate to CircleCI2.0 and Add AppVeyor for master-only branch #11605

Merged
merged 3 commits into from Nov 28, 2017

Conversation

raphamorim
Copy link
Contributor

@raphamorim raphamorim commented Nov 20, 2017

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
Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

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

worth using 8-alpine?

Copy link
Contributor Author

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/) ?

command: ./scripts/circleci/test_entry_point.sh

- save_cache:
key: node-modules-{{ checksum "yarn.lock" }}
Copy link
Contributor

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

@jquense
Copy link
Contributor

jquense commented Nov 21, 2017

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

@raphamorim
Copy link
Contributor Author

No problem at all @jquense.

but worth thinking about while the pieces are being moved

I totally agree.

@serima serima mentioned this pull request Nov 22, 2017
@gaearon
Copy link
Collaborator

gaearon commented Nov 22, 2017

I created an AppVeyor project. What's the next step?

@raphamorim
Copy link
Contributor Author

raphamorim commented Nov 22, 2017

@gaearon note that AppVeyor it's running only for master branch 👍
https://ci.appveyor.com/project/gaearon/react/history

@raphamorim
Copy link
Contributor Author

raphamorim commented Nov 22, 2017

I think that's it.

@raphamorim
Copy link
Contributor Author

raphamorim commented Nov 27, 2017

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these irrelevant now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 :octocat:

@raphamorim
Copy link
Contributor Author

raphamorim commented Nov 28, 2017

Are these irrelevant now?

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 README.md

BTW: Note the warning on 1.0 build (e.g: https://circleci.com/gh/facebook/react/7853#tests/containers/3):
circle.yml specified cache directory: /home/ubuntu/.yarn-cache but it does not exist

@gaearon gaearon merged commit a2b6b6b into facebook:master Nov 28, 2017
@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2017

Looks good to me. Thanks.

@felicianotech
Copy link

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.

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

Successfully merging this pull request may close these issues.

None yet

5 participants