-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
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.
This is fantastic, thank you for starting this work. I left a bunch of comments inline.
In addition:
- Can we move the
.cfg
files toci/kokoro/docker/
? - The CI compiles the code, but I do not think we test the
install
target for our libraries. We should, but that can be in a future PR.
Note that testing install
is more than just running make install
, we also want to verify the installed headers and libraries are usable, i.e., we need to compile, link and run a test program against said libraries.
@scotthart just FYI, note how we now have 3 copies of the scripts to drive the CI builds. A better way does not immediately come to mind, but this is what I mean by we need to design that part of the multi-repository thing.
@coryan Thanks, I think I addressed all the comments. PTAL |
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.
A couple of nits, please fix before merging.
Do we need a common.cfg file like this one?
https://github.com/googleapis/google-cloud-cpp/blob/master/ci/kokoro/docker/common.cfg
@coryan Thanks, I added common.cfg, merging |
Adding some basic kokoro builds for now. We'll add more tests with #2 and #3.