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

Simplify CI detection #50661

Merged
merged 2 commits into from Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Expand Up @@ -46,7 +46,7 @@ jobs:
- run: npm ci

- name: Linter
run: npm run lint:ci
run: npm run lint

browser-integration:
runs-on: ubuntu-latest
Expand Down
3 changes: 0 additions & 3 deletions Gulpfile.js
Expand Up @@ -372,9 +372,6 @@ const lint = eslint(".");
lint.displayName = "lint";
task("lint", lint);
task("lint").description = "Runs eslint on the compiler and scripts sources.";
task("lint").flags = {
" --ci": "Runs eslint additional rules",
Copy link
Member Author

Choose a reason for hiding this comment

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

For posterity, there are no additional rules; all rules already run. Probably this is a leftover from tslint or something.

};

const buildCancellationToken = () => buildProject("src/cancellationToken");
const cleanCancellationToken = () => cleanProject("src/cancellationToken");
Expand Down
1 change: 0 additions & 1 deletion package.json
Expand Up @@ -109,7 +109,6 @@
"clean": "gulp clean",
"gulp": "gulp",
"lint": "gulp lint",
"lint:ci": "gulp lint --ci",
"setup-hooks": "node scripts/link-hooks.js"
},
"browser": {
Expand Down
10 changes: 7 additions & 3 deletions scripts/build/options.js
Expand Up @@ -2,9 +2,11 @@
const minimist = require("minimist");
const os = require("os");

const ci = ["1", "true"].includes(process.env.CI);
Copy link
Member

Choose a reason for hiding this comment

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

This gets set by the workflow or something, right? It's not something we set elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct; GHA sets CI=1 or similar.


/** @type {CommandLineOptions} */
module.exports = minimist(process.argv.slice(2), {
boolean: ["dirty", "light", "colors", "lint", "lkg", "soft", "fix", "failed", "keepFailed", "force", "built"],
boolean: ["dirty", "light", "colors", "lint", "lkg", "soft", "fix", "failed", "keepFailed", "force", "built", "ci"],
string: ["browser", "tests", "break", "host", "reporter", "stackTraceLimit", "timeout", "shards", "shardId"],
alias: {
/* eslint-disable quote-props */
Expand Down Expand Up @@ -33,12 +35,13 @@ module.exports = minimist(process.argv.slice(2), {
reporter: process.env.reporter || process.env.r,
lint: process.env.lint || true,
fix: process.env.fix || process.env.f,
workers: process.env.workerCount || ((os.cpus().length - (process.env.CI ? 0 : 1)) || 1),
workers: process.env.workerCount || ((os.cpus().length - (ci ? 0 : 1)) || 1),
failed: false,
keepFailed: false,
lkg: true,
dirty: false,
built: false
built: false,
ci,
}
});

Expand Down Expand Up @@ -67,6 +70,7 @@ if (module.exports.built) {
* @property {string|number} timeout
* @property {boolean} failed
* @property {boolean} keepFailed
* @property {boolean} ci
*
* @typedef {import("minimist").ParsedArgs & TypedOptions} CommandLineOptions
*/
Expand Down
2 changes: 1 addition & 1 deletion scripts/build/tests.js
Expand Up @@ -123,7 +123,7 @@ async function runConsoleTests(runJs, defaultReporter, runInParallel, watchMode)
errorStatus = exitCode;
error = new Error(`Process exited with status code ${errorStatus}.`);
}
else if (process.env.CI === "true") {
else if (cmdLineOptions.ci) {
// finally, do a sanity check and build the compiler with the built version of itself
log.info("Starting sanity check build...");
// Cleanup everything except lint rules (we'll need those later and would rather not waste time rebuilding them)
Expand Down