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

Circle 2.0 setup requires further work #16306

Closed
grabbou opened this issue Oct 11, 2017 · 12 comments
Closed

Circle 2.0 setup requires further work #16306

grabbou opened this issue Oct 11, 2017 · 12 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@grabbou
Copy link
Contributor

grabbou commented Oct 11, 2017

Is this a bug report?

Bug

Have you read the Contributing Guidelines?

Yes

Environment

Not related, issue is about CI.

Steps to Reproduce

Checkout stable branch of master and try releasing.

Expected Behavior

CircleCI builds android, js and website in parallel. After all tasks have finished, deploy_website and deploy_npm jobs are run.

Actual Behavior

CircleCI builds android, js and website not as jobs, but as separate workflows. That results in those 3 being unaware of each other. That makes it hard to run something as a result of successful test. A good example is missing npm publish. I tried adding it and couldn't find the right place. There's no hook to wait for all workflows to finish with success. You can't run it on deploy_website job because android tests can fail in the meantime (that did happen to me during my tests).

Also, as in this example (https://circleci.com/gh/facebook/react-native/22571) website is not deployed despite all conditions being met.

How can this be fixed

We can add deploy_npm job after deploy_website and mark it as the one that requires manual approval. That means person releasing new version (me in particular) would have to double check that all tests are green and hit the deploy.

Alternatively, npm publish permissions has to be given to someone who will be running this command locally.

@hramos
Copy link
Contributor

hramos commented Oct 11, 2017

A website deploy should only happen on stable and master branches. If the step is being skipped on those branches, it's because the test-website step is failing. The deploy should happen regardless of failures on other unrelated steps (such as test-node-*, test-android).

Is the deploy-website step being skipped when test-website is successful? Or is there something we need to fix in test-website?

@grabbou
Copy link
Contributor Author

grabbou commented Oct 11, 2017

CC: @ide and @hramos who were involved in Circle 2.0 migration. Right now, I have reverted this change and built on 1.0

@grabbou
Copy link
Contributor Author

grabbou commented Oct 11, 2017

The deploy should happen regardless of failures on other unrelated steps (such as test-node-*, test-android).

Yes, @hramos, you are right! That's indeed same behaviour that we had in 1.x. Unfortunately, please look at the CircleCI - here's an example of a skipped build https://circleci.com/gh/facebook/react-native/22571 (and I think in this case it shouldn't).

My issue is related to fact that ./scripts/publish-npm.js has been removed and there's no way for me to release a successful build now.

@grabbou
Copy link
Contributor Author

grabbou commented Oct 11, 2017

There's no immediate need to work on it as I've restored to 1.0 on 0.50-stable. This will be used only for purposes of releasing new version.

I will use this branch to roll out a fix and create a pipeline that satisfies our open source requirements. We will investigate further workflow / jobs setup to see what can be done in order to improve it.

@ramshankerji
Copy link

ramshankerji commented Oct 12, 2017

NPM still has not received 0.5 as of now.
Need to publish it manually till the fix is worked out?

@grabbou
Copy link
Contributor Author

grabbou commented Oct 12, 2017 via email

@ramshankerji
Copy link

Rechecked. I see following on NPM site in relevant section.

react-native-bot react-native-bot published 3 days ago
0.49.3 is the latest of 216 releases
github.com/facebook/react-native
BSD-3-Clause

@grabbou
Copy link
Contributor Author

grabbou commented Oct 12, 2017 via email

@hramos
Copy link
Contributor

hramos commented Oct 12, 2017

The npm deploy step is missing from the Circle CI 2.0 config as Mike said earlier. Will open a PR to add it back.

@hramos
Copy link
Contributor

hramos commented Oct 12, 2017

Opened #16340, but Circle is not yet running a build for it.

@hramos
Copy link
Contributor

hramos commented Oct 16, 2017

#16350 landed. We now wait for approval when running builds on master and stable releases. This should allow you to selectively deploy to npm when a new release is cut.

@grabbou
Copy link
Contributor Author

grabbou commented Oct 17, 2017

I believe this can be closed now.

@grabbou grabbou closed this as completed Oct 17, 2017
@facebook facebook locked as resolved and limited conversation to collaborators Oct 17, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants