-
Notifications
You must be signed in to change notification settings - Fork 616
core: ci scripts overhaul #1597
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
Could you go comment-crazy in the files, in terms of headings to each major file / function on what its purpose is? |
|
||
if (!modules.length) { | ||
echo('No code changes found, exiting early.'); | ||
exit(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
var mod = new Module(MODULE_NAME); | ||
|
||
mod.install(); | ||
mod.runSystemTests(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
I think we might need an overhaul on what a build is and what a release is. Since Travis doesn't publish to npm for us anymore, what distinguishes the two? |
Topics that would be awesome if they were covered in the "Developer's Documentation" code comments:
I have some more code-specific confusions, but I'll wait to see something like ^^ before I confuse us both by asking. |
@stephenplusplus I went ahead and added a bunch of docs/comments to build.js - PTAL and let me know if you're cool with the format, then I'll do the others. |
* - For each module that has updates, we'll install their dependencies and run | ||
* their unit tests. | ||
* | ||
* - Assuming all the unit tests pass, we'll then attempt to get code coverage |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
* | ||
* - Assuming all system tests pass, we'll create symlinks for each module | ||
* via `npm link`. We'll then search for any modules that have the updated | ||
* modules listed as dependencies or devDependencies. For each dependent |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
* | ||
* - We'll then check to see if this build requires a documentation update. | ||
* This is determined by the `TEST_ONLY` environment variable. This is set | ||
* in the .travis.yml file. If this build does not require a documentation |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
They're beautiful 😢 |
Is there anything left to do on this before I review? |
@stephenplusplus I think it's good to go! |
Closes #1579
Don't merge until after #1606
This PR aims to do 2 major things