-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
build,test: add .ci.yml for containered tests (WIP proposal) #30057
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
Conversation
38e4c59 to
f7641a8
Compare
|
Documentation for this PR is currently @ nodejs/build#1982, this PR serves as part of that proposal. "Show all checks" below in GitHub's status checks shows some of what this is doing. |
9bbecd5 to
a64c93f
Compare
b8859e1 to
fcc93ea
Compare
|
Also any thought on add |
That'd be fine, these quick checks, like the linter, are ideal to add in here since they are cheap. You just need to have a validation to make sure it's acutally worked. Returning a non-zero exit code is one thing, but has it done what it's supposed to? Maybe you could work up a proposal for how to run and check that? btw, putting CMake into the standard Ubuntu 18.04 container, like Ninja is, would make it possible to run full CMake builds with this too, which might be a step in a positive direction. |
.ci.yml
Outdated
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.
Is it okay to write those two as env variable in docker image ?
|
Impressive! |
|
@rvagg Is there any left work to do ? Can we mark this as ready ? |
@gengjiawen It's not ready, see nodejs/build#1982 (comment) |
.ci.yml
Outdated
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.
Shouldn't PYTHON be defined before running make instead of after?
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.
You can do either.
PYTHON=python2 make lint-ciSets the PYTHON environment variable in the shell and runs make, which turns environment variables into make variables: https://www.gnu.org/software/make/manual/html_node/Environment.html#Environment
make lint-ci PYTHON=python2Overrides the make variable from the command line: https://www.gnu.org/software/make/manual/html_node/Overriding.html
|
Ping @rvagg |
1 similar comment
|
Ping @rvagg |
8ae28ff to
2935f72
Compare
|
Ping @rvagg (again :-) ...) |
fcc93ea to
290ae13
Compare
290ae13 to
b52aa4b
Compare
|
I've tried and tried to make this work but there's a weird interaction between |
I'm working up a proposal for Build, this can be safely ignored for now, I'll be getting the rest of it up and documented in nodejs/build for further discussion.