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

Major gulp refactor: blueprint.defineTaskGroup() #617

Merged
merged 14 commits into from
Feb 8, 2017
Prev Previous commit
Next Next commit
swap taskFn args order, add dependencies arg
for tasks that care about project dependencies 😄
  • Loading branch information
Gilad Gray committed Feb 3, 2017
commit bce87b7f21bf1fc57e5642cfad3e2d4fae6eb6d3
4 changes: 2 additions & 2 deletions gulp/dist.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = (blueprint, gulp) => {
blueprint.taskGroup({
block: "typescript",
name: "bundle",
}, (taskName, project) => {
}, (project, taskName) => {
gulp.task(taskName, (done) => {
webpack(
webpackConfig.generateWebpackBundleConfig(project),
Expand All @@ -26,7 +26,7 @@ module.exports = (blueprint, gulp) => {
blueprint.taskGroup({
block: "all",
name: "test-dist",
}, (taskName, project) => {
}, (project, taskName) => {
gulp.task(taskName, () => {
const pkgJson = require(path.resolve(project.cwd, "package.json"));
const promises = ["main", "style", "typings"]
Expand Down
12 changes: 7 additions & 5 deletions gulp/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,24 @@ module.exports = (gulp, config) => {
* 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 `(taskName, project)`. The task
* name is of the format `[name]-[project.id]`.
* 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
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of "block", I propose another name: "scope"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

love it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually i'm less into scope now. config block sounds better than config scope, as does projectsWithBlock vs projectsWithScope.

i am open to a new name though, just don't think we've found it yet.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 projectsWithScope sounds fine to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 `(taskName, project)`
* @param {Function} taskFn called for each project containing block with `(project, taskName, depTaskNames)`
*/
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, project.id].join("-");
taskFn(taskName, project);
const taskName = [name, id].join("-");
const depNames = dependencies.map((dep) => [name, dep].join("-"));
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that dep and id both project IDs in practice (e.g. "core", "datetime", "table"). It'd be helpful to name dep to depId IMO, just so it's clear that the names we're building on this line and the line above both follow the same scheme.

taskFn(project, taskName, depNames);
return taskName;
});

Expand Down
2 changes: 1 addition & 1 deletion gulp/isotest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module.exports = (blueprint, gulp, plugins) => {
blueprint.taskGroup({
block: "isotest",
parallel: false,
}, (taskName, project) => {
}, (project, taskName) => {
gulp.task(taskName, [`typescript-compile-${project.id}`], () => {
return gulp.src(project.cwd + "test.iso/**/*")
Copy link
Contributor

Choose a reason for hiding this comment

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

@giladgray just gonna comment on every little thing until I build up a more solid understanding of this whole build system. There's a minor change in behavior here, yes (${project.cwd}/test/isotest.js => ${project.cwd}/test.io/**/*)? Is the reasoning that we expect to add more isomorphic test files later, so might as well support anything in that directory moving forward?

.pipe(plugins.mocha());
Expand Down
2 changes: 1 addition & 1 deletion gulp/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = (blueprint, gulp, plugins) => {
blueprint.taskGroup({
block: "karma",
parallel: false,
}, (taskName, project) => {
}, (project, taskName) => {
gulp.task(taskName, (done) => {
const server = new karma.Server(createConfig(project), done);
return server.start();
Expand Down