Skip to content

Add CircleCI config #131

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

Merged
merged 3 commits into from
Oct 11, 2017
Merged

Add CircleCI config #131

merged 3 commits into from
Oct 11, 2017

Conversation

jxom
Copy link
Contributor

@jxom jxom commented Oct 11, 2017

This intends to fix the third and final item for #67.

Basically just runs the ci-check script upon every build, as well as caching the project dependencies 🤗.

@reactjs-bot
Copy link

Deploy preview ready!

Built with commit 086322a

https://deploy-preview-131--reactjs.netlify.com

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Sweet! Thanks!

Left a comment about yarn.lock file. What are your thoughts?

- image: circleci/node:latest
steps:
- checkout
- restore_cache:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a step to explicitly install Yarn if it's not already installed? The React repo's circle.yml file has this:

  pre:
    # This is equivalent to $TRAVIS_COMMIT_RANGE
    - echo $CIRCLE_COMPARE_URL | cut -d/ -f7
    # install yarn if it's not already installed
    - |
      if [[ ! -e ~/.yarn/bin/yarn || $(yarn --version) != "${YARN_VERSION}" ]]; then
        curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version $YARN_VERSION
      fi

Maybe this isn't necessary with the 2.0 API. The 1.0 API docs have a section about how to enable Yarn which the 2.0 docs don't have. Maybe it just works out of the box now? 😁 Let's try and see!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure it’s already installed on v2

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, seems like that's the case.

- checkout
- restore_cache:
keys:
- dependencies-{{ checksum "package.json" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should checksum "yarn.lock" instead? Seems like that may be better?

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 agree!

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Let's iterate on this. I'll move forward with the package.json checksum now and we can chat more about the lockfile alternative.

@bvaughn bvaughn merged commit c4aaa15 into reactjs:master Oct 11, 2017
@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

Looks good so far 👍

The first build commit (c4aaa15) failed: https://circleci.com/gh/reactjs/reactjs.org/1

This was because a Prettier issue had slipped in. I fixed it (42372b2) and the second build seems happy: https://circleci.com/gh/reactjs/reactjs.org/2

😄

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

We've also got Circle building PRs from forks now too (#134, #135)

jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020
…js#131)

* translate navbar

* Update content/docs/nav.yml

Co-Authored-By: WeslleyNasRocha <[email protected]>

* Apply suggestions from code review

Co-Authored-By: WeslleyNasRocha <[email protected]>
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
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.

4 participants