-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Major gulp refactor: blueprint.defineTaskGroup() #617
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
Changes from 4 commits
661436c
e52a0f1
bce87b7
7c9755b
fad2c05
b3c09e3
183c315
0044b9b
b13f70c
6952e15
9126e3d
aba0117
aeb0fdd
2629f96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,8 @@ module.exports = (blueprint, gulp, plugins) => { | |
| return del(cleanDirs, { force: true }); | ||
| }); | ||
|
|
||
| gulp.task("tslint", () => ( | ||
| gulp.src(["*.js", "gulp/**/*.js", "packages/*/*.js"]) | ||
| gulp.task("tslint-gulp", () => ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the motivation behind this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok so: the previous iteration of this task ran tslint on all this new iteration runs tslint on the gulp code only because JS files in packages are now linted by
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha 👍 |
||
| gulp.src(["*.js", "gulp/**/*.js"]) | ||
| .pipe(plugins.tslint({ formatter: "verbose" })) | ||
| .pipe(plugins.tslint.report()) | ||
| .pipe(plugins.count("## javascript files linted")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,9 @@ | |
| */ | ||
| "use strict"; | ||
|
|
||
| const rs = require("run-sequence"); | ||
| const path = require("path"); | ||
| const util = require("util"); | ||
| const plugins = require("gulp-load-plugins")(); | ||
|
|
||
| /** | ||
|
|
@@ -43,17 +45,32 @@ module.exports = (gulp, config) => { | |
| }, | ||
|
|
||
| /** | ||
| * Returns an array of task names of the format [prefix]-[...prefixes]-[project] | ||
| * for every project with the given block. | ||
| * @param block {string} name of project config block | ||
| * @param prefix {string} first prefix, defaults to block name | ||
| * @param prefixes {string[]} additional prefixes before project name | ||
| * @returns {string[]} | ||
| * Define a group of tasks for projects with the given config block. | ||
| * The special block `"all"` will operate on all projects. | ||
| * The `block` is used as the task name, unless `name` is explicitly defined. | ||
| * The `taskFn` is called for each matched project with `(project, taskName, depTaskNames)`. | ||
| * The task name is of the format `[name]-[project.id]`. | ||
| * Finally, a "group task" is defined with the base name that runs all the project tasks. | ||
| * This group task can be configured to run in parallel or in sequence. | ||
| * @param {{block: string, name?: string, parallel?: boolean}} options | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of "block", I propose another name: "scope"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. love it!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually i'm less into i am open to a new name though, just don't think we've found it yet.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. block really doesn't mean anything. what do you mean by "config block sounds better than config scope"? also
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is a scope? |
||
| * @param {Function} taskFn called for each project containing block with `(project, taskName, depTaskNames)` | ||
| */ | ||
| taskMapper(block, prefix = block, ...prefixes) { | ||
| return blueprint | ||
| .projectsWithBlock(block) | ||
| .map(({ id }) => [prefix, ...prefixes, id].join("-")); | ||
| taskGroup(options, taskFn) { | ||
| const { block, name = block, parallel = true } = options; | ||
|
|
||
| const projects = (block === "all") ? blueprint.projects : blueprint.projectsWithBlock(block); | ||
|
|
||
| const taskNames = projects.map((project) => { | ||
| const { dependencies = [], id } = project; | ||
| // string name is combined with block; array name ignores/overrides block | ||
| const taskName = [name, id].join("-"); | ||
| const depNames = dependencies.map((dep) => [name, dep].join("-")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears that |
||
| taskFn(project, taskName, depNames); | ||
| return taskName; | ||
| }); | ||
|
|
||
| // can run tasks in series when it's preferable to keep output separate | ||
| gulp.task(name, parallel ? taskNames : (done) => rs(...taskNames, done)); | ||
| }, | ||
| }, config); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,10 +4,13 @@ | |
| "use strict"; | ||
|
|
||
| module.exports = (blueprint, gulp, plugins) => { | ||
| const path = require("path"); | ||
|
|
||
| blueprint.task("isotest", "mocha", ["typescript-compile-*"], (project) => { | ||
| return gulp.src(path.join(project.cwd, "test", "isotest.js")) | ||
| .pipe(plugins.mocha()); | ||
| blueprint.taskGroup({ | ||
| block: "isotest", | ||
| parallel: false, | ||
| }, (project, taskName) => { | ||
| gulp.task(taskName, [`typescript-compile-${project.id}`], () => { | ||
| return gulp.src(project.cwd + "test.iso/**/*") | ||
|
||
| .pipe(plugins.mocha()); | ||
| }); | ||
| }); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,14 +8,13 @@ module.exports = (blueprint, gulp) => { | |
| const webpackConfig = require("./util/webpack-config"); | ||
|
|
||
| const docsProject = blueprint.findProject("docs"); | ||
|
|
||
| const configuration = webpackConfig.generateWebpackTypescriptConfig(docsProject); | ||
|
|
||
| gulp.task("webpack-compile-docs", ["docs"], (callback) => { | ||
| gulp.task("webpack-docs", ["docs"], (callback) => { | ||
| webpack(configuration, webpackConfig.webpackDone(callback)); | ||
| }); | ||
|
|
||
| gulp.task("webpack-compile-w-docs", (callback) => { // eslint-disable-line no-unused-vars | ||
| gulp.task("webpack-docs-watch", (callback) => { | ||
| // rely on editor for compiler errors during development--this results in _massive_ speed increase | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we not need the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are not using eslint anymore! it's all TSLint |
||
| configuration.ts.transpileOnly = true; | ||
| // never invoke callback so it runs forever! | ||
|
|
||
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.
So this check is gone now. In the new code, seems like omitting a key/blockname (e.g.
copy) from a project config would causeprojectsWithBlockto return an empty array in blueprint.taskGroup, which would then define agulp.taskthat simply executes an empty set of sub-tasks—effectively a no-op. Is that behavior drastically different from this special no-op case above that used to be here? Was it ever particularly helpful or necessary?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.
blueprint.taskalways generated a root task that depended on corresponding tasks in other projects. socopy-files-docsdepended on (the existence of)copy-files-core, which is why we had to add the no-op shortcut. this was awkward because there wasn't actually a dependency between the copy tasks (like, they're completely separate operations), whereas there is a real dependency for the typescript tasks (core must be compiled before datetime).in this new world, the
copytask simply ignores thedepTaskNamesargument and voila, packages can be copied without any dependencies, as they should be!