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

[CI] Publish to npm on stable and master branches #16348

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 123 additions & 23 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@ aliases:
- website/node_modules
key: v1-website-dependencies-{{ .Branch }}-{{ checksum "website/package.json" }}

- &restore-cache-danger
- &restore-cache-analysis
keys:
- v1-danger-dependencies-{{ .Branch }}-{{ checksum "danger/package.json" }}
- v1-analysis-dependencies-{{ .Branch }}-{{ checksum "package.json" }}{{ checksum "danger/package.json" }}
# Fallback in case checksum fails
- v1-danger-dependencies-{{ .Branch }}-
- &save-cache-danger
- v1-analysis-dependencies-{{ .Branch }}-
- &save-cache-analysis
paths:
- danger/node_modules
key: v1-danger-dependencies-{{ .Branch }}-{{ checksum "danger/package.json" }}
- node_modules
key: v1-analysis-dependencies-{{ .Branch }}-{{ checksum "package.json" }}{{ checksum "danger/package.json" }}

- &restore-cache-android-packages
keys:
Expand Down Expand Up @@ -90,29 +91,29 @@ jobs:
- image: circleci/node:8
steps:
- checkout
- run: npm install --no-package-lock
- run:
name: Install Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
- run: |
npm test -- --maxWorkers=2
npm run lint
npm run flow -- check
# eslint - doesn't run on non-PR builds
- run:
name: Analyze Code
command: |
if [ -n "$CIRCLE_PR_NUMBER" ]; then
npm install github@0.2.4
cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js
else
echo "Skipping code analysis."
fi

test-node-6:
<<: *defaults
docker:
- image: circleci/node:6.11.0
steps:
- checkout
- run: npm install
- run:
name: Install Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install
- run: |
npm test -- --maxWorkers=2
npm run lint
Expand All @@ -124,12 +125,17 @@ jobs:
- image: circleci/node:4.8.4
steps:
- checkout
- run: npm install
- run:
name: Install Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install
- run: |
npm test -- --maxWorkers=2
npm run lint
npm run flow -- check

test-website:
<<: *defaults
docker:
Expand All @@ -140,6 +146,8 @@ jobs:
name: Install Dependencies
command: |
cd website
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
- run:
name: Test Website
Expand All @@ -160,6 +168,8 @@ jobs:
name: Install Dependencies
command: |
cd website
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
- run:
name: Build and Deploy Static Website
Expand All @@ -168,7 +178,6 @@ jobs:
git config --global user.email "reactjs-bot@users.noreply.github.com"
git config --global user.name "Website Deployment Script"
echo "machine github.com login reactjs-bot password $GITHUB_TOKEN" > ~/.netrc

echo "Deploying website..."
cd website && GIT_USER=reactjs-bot npm run gh-pages
else
Expand All @@ -181,7 +190,12 @@ jobs:
- image: circleci/node:8
steps:
- checkout
- run: npm install --no-package-lock
- run:
name: Install Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
- run:
name: Build JavaScript Bundle
command: node local-cli/cli.js bundle --max-workers 2 --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js
Expand Down Expand Up @@ -258,7 +272,12 @@ jobs:
command: |
curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash -
sudo apt-get install -y nodejs
- run: npm install
- run:
name: Install Node Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
# - restore-cache: *restore-cache-watchman
# - run:
# name: Install Watchman Dependencies
Expand Down Expand Up @@ -346,6 +365,72 @@ jobs:
- store_artifacts:
path: ~/junit

analyze-pull-request:
<<: *defaults
docker:
- image: circleci/node:8
steps:
- checkout
- restore-cache: *restore-cache-analysis
- run:
name: Install Dependencies
command: |
if [ -n "$CIRCLE_PULL_REQUEST" ]; then
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
npm install github@0.2.4
cd danger
npm install --no-package-lock
else
echo "Skipping dependency installation."
fi
- save-cache: *save-cache-analysis
# Run Danger
- run:
name: Analyze Pull Request
command: |
if [ -n "$CIRCLE_PULL_REQUEST" ]; then
cd danger && DANGER_GITHUB_API_TOKEN="e622517d9f1136ea8900""07c6373666312cdfaa69" npm run danger
else
echo "Skipping pull request analysis."
fi
when: always
Copy link
Contributor

Choose a reason for hiding this comment

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

Wish there was a way to run on pull request builds only in a more declarative way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same.

# Run eslint
- run:
name: Analyze Code
command: |
if [ -n "$CIRCLE_PULL_REQUEST" ]; then
cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js
else
echo "Skipping code analysis."
fi

publish-npm:
<<: *defaults
docker:
- image: circleci/node:8
steps:
- checkout
- run:
name: Install Dependencies
command: |
npm config set spin=false
npm config set progress=false
npm install --no-package-lock
- run:
name: Publish React Native Package
command: |
if [ -z "$CIRCLE_PULL_REQUEST" ]; then
echo "//registry.npmjs.org/:_authToken=${CIRCLE_NPM_TOKEN}" > ~/.npmrc
git config --global user.email "reactjs-bot@users.noreply.github.com"
git config --global user.name "Website Deployment Script"
echo "machine github.com login reactjs-bot password $GITHUB_TOKEN" > ~/.netrc
node ./scripts/publish-npm.js
else
echo "Skipping publication."
fi

# Workflows enables us to run multiple jobs in parallel
workflows:
version: 2
Expand All @@ -366,6 +451,21 @@ workflows:
only:
- /.*-stable/
- master
deploy:
jobs:
- publish-npm
branches:
only:
- /.*-stable/
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

This will publish ALWAYS from stable regardless of the tests. We should either wait for other workflows programatically to complete (no API yet that I am aware that allows it - you can only queue up jobs within particular workflow) or make this job "manual", meaning that I have to go to CircleCI and manually click "Deploy" . I actually believe that would be better approach. Many times, CI builds fail because of Flow, flakiness or something. Having manual way to approve build regardless of the failure would make it easier to avoid such issues in the future.

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 leaning towards taking advantage of the manual approval, for the reasons mentioned. I need to look into who would have the necessary permission to approve the deploy to go out - I wouldn't want to hold you back from pushing out a release if this is only available to FB employees. (It's midnight here, I'll check up on this in the morning)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. I think write access might be needed. If I need to undergo special security constraints here or something (like use a separate account), happy to comply with it. I think given my past experience, ti would be great to not wait for FB since you guys are pretty busy anyway!

analyze:
jobs:
- analyze-pull-request
branches:
ignore:
- master
- gh-pages
- /.*-stable/
test_android:
jobs:
- build-js-bundle:
Expand All @@ -374,4 +474,4 @@ workflows:
ignore: gh-pages
- test-android:
requires:
- build-js-bundle
- build-js-bundle
18 changes: 9 additions & 9 deletions danger/dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ if (addsDocs || editsDocs) {
message(`${title} - <i>${idea}</i>`);

// Add the Documentation label via bot, as @facebook-open-source-bot does not have write privileges on the repo.
markdown('@facebook-github-bot label Documentation');
// markdown('@facebook-github-bot label Documentation');
}

const isBlogFile = path => includes(path, 'blog/');
Expand All @@ -53,7 +53,7 @@ if (editsBlogPost) {
// Fails if the description is too short.
if (danger.github.pr.body.length < 10) {
fail(':grey_question: This pull request needs a description.');
markdown('@facebook-github-bot label Needs more information');
// markdown('@facebook-github-bot label Needs more information');
}

// Warns if the PR title contains [WIP]
Expand Down Expand Up @@ -83,23 +83,23 @@ if (!includesTestPlan && gettingStartedChanged) {
const title = ':clipboard: Test Plan';
const idea = 'This PR appears to be missing a Test Plan.';
warn(`${title} - <i>${idea}</i>`);
markdown('@facebook-github-bot label Needs more information');
// markdown('@facebook-github-bot label Needs more information');
}
// Doc edits rarely require a test plan. We'll trust the reviewer to push back if one is needed.
if (!includesTestPlan && !editsDocs) {
const title = ':clipboard: Test Plan';
const idea = 'This PR appears to be missing a Test Plan.';
warn(`${title} - <i>${idea}</i>`);
markdown('@facebook-github-bot label Needs more information');
// markdown('@facebook-github-bot label Needs more information');
}

// Tags PRs that have been submitted by a core contributor.
// TODO: Switch to using an actual MAINTAINERS file.
const taskforce = fs.readFileSync('../bots/IssueCommands.txt', 'utf8').split('\n')[0].split(':')[1];
const isSubmittedByTaskforce = includes(taskforce, danger.github.pr.user.login);
if (isSubmittedByTaskforce) {
markdown('@facebook-github-bot label Core Team');
}
// if (isSubmittedByTaskforce) {
// markdown('@facebook-github-bot label Core Team');
// }

// Tags big PRs
var bigPRThreshold = 600;
Expand All @@ -108,14 +108,14 @@ if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
const idea = `This PR is extremely unlikely to get reviewed because it touches ${danger.github.pr.additions + danger.github.pr.deletions} lines.`;
warn(`${title} - <i>${idea}</i>`);

markdown('@facebook-github-bot large-pr');
// markdown('@facebook-github-bot large-pr');
}
if (danger.git.modified_files + danger.git.added_files + danger.git.deleted_files > bigPRThreshold) {
const title = ':exclamation: Big PR';
const idea = `This PR is extremely unlikely to get reviewed because it touches ${danger.git.modified_files + danger.git.added_files + danger.git.deleted_files} files.`;
warn(`${title} - <i>${idea}</i>`);

markdown('@facebook-github-bot large-pr');
// markdown('@facebook-github-bot large-pr');
}

// Warns if the bots whitelist file is updated.
Expand Down