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

Optimize CI workflow #2331

Merged
merged 7 commits into from
Nov 17, 2017
Merged

Optimize CI workflow #2331

merged 7 commits into from
Nov 17, 2017

Conversation

Hypnosphi
Copy link
Member

Issue: #2179

@codecov
Copy link

codecov bot commented Nov 16, 2017

Codecov Report

Merging #2331 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2331   +/-   ##
=======================================
  Coverage   21.19%   21.19%           
=======================================
  Files         283      283           
  Lines        6156     6156           
  Branches      724      731    +7     
=======================================
  Hits         1305     1305           
+ Misses       4301     4299    -2     
- Partials      550      552    +2
Impacted Files Coverage Δ
app/react/src/server/config/babel.js 0% <0%> (-100%) ⬇️
app/react/src/server/babel_config.js 0% <0%> (-77.42%) ⬇️
app/vue/src/server/utils.js 0% <0%> (-53.58%) ⬇️
addons/a11y/src/components/Report/Tags.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/PropForm.js 9.09% <0%> (ø) ⬆️
addons/knobs/src/components/types/Boolean.js 11.62% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/layout/usplit.js 38.7% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 30.98% <0%> (ø) ⬆️
addons/a11y/src/components/Tabs.js 0% <0%> (ø) ⬆️
addons/a11y/src/A11yManager.js 0% <0%> (ø) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ead3002...ee90082. Read the comment docs.

@Hypnosphi Hypnosphi added the maintenance User-facing maintenance tasks label Nov 16, 2017
@Hypnosphi
Copy link
Member Author

I made all the other jobs depend on build. This makes overall workflow time for a particular commit a bit longer, but significantly diminishes total time spent on agents, which means that builds will spend less time waiting in queue.

before: https://circleci.com/workflow-run/c7e6c92c-90f4-4746-8b4a-d70c21cf4850
after: https://circleci.com/workflow-run/5317bb4f-c4e9-4163-917a-ec4885f97043

@Hypnosphi
Copy link
Member Author

A further optimisation would be to move the docs and react native bootstrapping from build to the corresponding jobs, so that example-kitchen-sinks don't have to wait for it. Will try it now

@Hypnosphi
Copy link
Member Author

After moving docs bootstrapping to its own job:
https://circleci.com/workflow-run/048fc41b-0150-4a32-9de1-5988e2112a8d

@Hypnosphi
Copy link
Member Author

After moving react-native bootstrapping to its own job:
https://circleci.com/workflow-run/ac85393a-553f-4fbd-9ae5-0a7738e1dcd2

The next step could be moving RN unit-tests to example-react-native (and probably renaming the latter to just react-native)

@Hypnosphi
Copy link
Member Author

And voila — after moving react-native tests to the corresponding job both overall workflow time and total agent time is significantly less:
https://circleci.com/workflow-run/2ca1fd0a-5edd-443d-b6d5-f3c22d669319

@Hypnosphi Hypnosphi requested a review from a team November 16, 2017 23:39
@Hypnosphi Hypnosphi changed the title Cache build job results to reuse them in other jobs Optimize CI workflow Nov 16, 2017
paths:
- addons
- app
- lib
Copy link
Member Author

Choose a reason for hiding this comment

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

This caches app, addons, and lib directories using current commit hash as a part of the cache key.

bootstrap --core transpiles all the packages in those directories (src to dist).

Ideally, it should be

paths:
  - addons/*/dist
  - app/*/dist
  - lib/*/dist

But unfortunately, wildcards are not supported in cached directories paths

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

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

This is soo awesome, thanks!

@Hypnosphi Hypnosphi merged commit acf815e into master Nov 17, 2017
@Hypnosphi Hypnosphi deleted the ci-caching branch November 17, 2017 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants