-
Notifications
You must be signed in to change notification settings - Fork 493
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
use matrix for CircleCI config #2864
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2864 +/- ##
==========================================
+ Coverage 47.28% 47.31% +0.02%
==========================================
Files 355 355
Lines 56901 56901
==========================================
+ Hits 26908 26923 +15
+ Misses 26958 26948 -10
+ Partials 3035 3030 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read through it and reviewed the CircleCI results. It seems good to me and it introduced me to yaml syntax for using &ref_name
and <<: *ref_name
. The extended use of matrices for the executors does reduce a lot of duplicate code.
I wonder how easy it will be for someone new to jump into and make changes to the config file later on and whether the YAML is enough or if we need any documentation on what we expect our CircleCI to currently do.
Yes, I also didn't know that YAML syntax either until I saw it recently in some CircleCI docs and then read CircleCI's YAML guide on Anchors and aliases. |
lgtm, here's a good simple article on the << & and * https://medium.com/@kinghuang/docker-compose-anchors-aliases-extensions-a1e4105d70bd |
@cce is the GIMME change in the second file on purpose? what does it do exactly? |
The original build.sh was downloading via curl the "gimme" script to ~/gimme.sh, so I changed it to check for a GIMME_INSTALL_DIR and use that, falling back to $HOME as before if it is not set. That made it possible for the matrix build not to be so dependent on the value of $HOME. |
Summary
Based on the matrix job @algojack added in #2749, this turns the other per-platform jobs into matrix jobs too. The most significant change in this PR is switching from building in the home directory (/home/circleci on Linux and /Users/distiller on Mac) to instead build out of /opt/cibuild to make it easier for the matrix jobs to share configuration.
Test Plan
It would be good to check that all the same tests are being run as before, perhaps by going through the build output manually or the artifacts (resulting JSON/XML files).