Add script for fully building public site #56
Conversation
| }, { | ||
| id: "datetime", | ||
| cwd: "packages/datetime", | ||
| dependencies: ["core"], | ||
| sass: true, | ||
| typescript: true, | ||
| karma: true, | ||
| copy: false, | ||
| }, { | ||
| id: "docs", | ||
| cwd: "packages/docs/", | ||
| dependencies: [ | ||
| // docs typescript "depends" on all other projects, but as it uses webpack entirely, | ||
| // that dependency is expressed by making `webpack` tasks depend on `typescript` tasks. |
There was a problem hiding this comment.
comment isn't entirely accurate now, is it? update wording please?
| @@ -11,19 +11,22 @@ const projects = [ | |||
| sass: true, | |||
| typescript: true, | |||
| karma: true, | |||
| copy: false, | |||
There was a problem hiding this comment.
maybe a note that these tasks must exist yada yada so we don't forget?
| blueprint.task("copy", "files", [], (project) => ( | ||
| blueprint.task("copy", "files", [], (project) => { | ||
| // allow for no-op on project dependencies | ||
| if (project.copy === false) { |
There was a problem hiding this comment.
honestly i'd prefer to support {} here instead of false cuz false sounds like "don't run this task" but that's not what will happen.
There was a problem hiding this comment.
I want to follow the convention in Gulpfile.js where you can switch on/off tasks with true/false
There was a problem hiding this comment.
@giladgray isn't that exactly what's happening? i.e. this task is not being run, it aborts early.
There was a problem hiding this comment.
essential distinction is that the task exists, it just does nothing. and it needs to exist because of how project dependencies work here.
copy: false suggests to me that the project wants to opt out of that task, which is precisely not what happens here.
There was a problem hiding this comment.
I don't see why that semantic distinction is meaningful -- this is getting pedantic
| return gulp.src(config.srcGlob(project, true)) | ||
| .pipe(plugins.sourcemaps.init()) | ||
| .pipe(sassCompiler) | ||
| .pipe(plugins.postcss(postcssPlugins, postcssOptions)) | ||
| .pipe(plugins.autoprefixer(config.autoprefixer)) |
There was a problem hiding this comment.
is this a good time to move autoprefixer into postcss too? there's a warning on install that this plugin is deprecated and should be done through postcss.
| * @param project {Object} current project | ||
| * @param paths {string[]} subdirectories | ||
| */ | ||
| destPath(project, ...paths) { |
There was a problem hiding this comment.
argh i was hoping that dest() would be enough, which is why it includes the gulp.dest call.
worth unwrapping that so we only need one of these methods?
There was a problem hiding this comment.
Meh, then we'd just have to wrap the calls like gulp.dest(blueprint.dest(project))
| postcssUrl({ url: "rebase" }), | ||
| // copy assets to dist folder, respecting rebase | ||
| postcssCopyAssets({ | ||
| pathTransform: (newPath, origPath, contents) => { |
There was a problem hiding this comment.
contents is unused.
also the => () syntax would obviate the need for return.
then it might all fit on one line!
There was a problem hiding this comment.
I prefer the current syntax; I think a one-liner would be overkill / too dense here.
| @@ -34,6 +42,7 @@ | |||
| "gulp-insert": "0.5.0", | |||
| "gulp-less": "3.1.0", | |||
| "gulp-load-plugins": "1.3.0", | |||
| "gulp-postcss": "^6.2.0", | |||
There was a problem hiding this comment.
exact deps all the way down please. no ^
There was a problem hiding this comment.
i aliased nid to npm install --save-dev --save-exact
|
only 251 artifacts exported!!!!!!!! (down from like 20,000 in some PRs) |
| // | ||
| // * Even if you don't have a copy task, you should add `copy: false` to run a | ||
| // no-op copy task. This allows other packages that depend on yours to contain | ||
| // a copy task without failing the gulp build. |
| // that dependency is expressed by making `webpack` tasks depend on `typescript` tasks. | ||
| "datetime" | ||
| // You must add your package to this dependency list if you have any | ||
| // examples in the docs. |
There was a problem hiding this comment.
time to add "table" now that it's merged?
* Add script for fully building public site * lint * PR comments * add table to build script deps
No description provided.