-
Notifications
You must be signed in to change notification settings - Fork 155
Conversation
4312e53
to
cefa68a
Compare
Includes unit testing script and bunch of unit testing configuration.
This is not including a README, which will come later.
974e60a
to
18b3878
Compare
|
||
1. Create the Schematic Engine, and pass in a Collection and Schematic loader. | ||
1. Understand and respect the Schematics metadata and dependencies between collections. Schematics can refer to dependencies, and it's the responsibility of the tool to honor those dependencies. The reference CLI uses NPM packages for its collections. | ||
1. Create the Options object. Options can be anything, but the schematics can specify a JSON Schema that should be respected. The reference CLI, for example, parse the arguments as a JSON object and validate it with the Schema specified by the collection. |
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.
parse
->parses
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.
Done.
The tooling API is composed of the following pieces: | ||
|
||
## Engine | ||
The `SchematicEngine` is responsible for loading and constructing `Collection`s and `Schematics`'. When creating an engine, the tooling provides an `EngineHost` interface that understands how to create a `CollectionDescription` from a name, and how to create a `Schematic |
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.
Unclosed backtick at `Schematic
* found in the LICENSE file at https://angular.io/license | ||
*/ | ||
|
||
export class BaseException extends Error { |
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.
This might not be a good idea in Ts 2.1+: https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
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.
It works in Node, I had that discussion with a lot of folks already.
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.
I assume you're building down to ES2015 and requiring Node 7+ (I think), then? Because the only problems come if you're targeting ES5. If that's the case, you just need to add Object.setPrototypeof(this, BaseException.prototype)
in the constructor
, and you'll be fine in any version of Node.
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.
Yes, we're targeting ES2015.
|
||
module.exports = function (config) { | ||
config.set({ | ||
basePath: '), |
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.
'),
->''
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.
Done. Not sure how that ended up here.
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.
Went through the big bits with Hans on a call, looks really good. Reference CLI has some really clean and descriptive usage patterns.
Great work on this Hans 👍
0c0a350
to
39f093d
Compare
Better separation of concern between tooling and schematic library. Engine, Collection and Schematic now take a generic type that can add tooling-specific metadata to each parts, in a type-safe manner. Context exposed to schematics is a context of <any, any>, while the tool should use TypedContext for better support internally. The Engine Host now only deals with Descriptions of collection and schematics, does not deal with actual implementation. Renamed CliEngineHost to NodeModulesEngineHost, since its kind of reusable if you only implement a NodeModules schematic tool. Tempted to move this into the schematics library as its highly reusable. Added tooling/ in the schematics package, which contains implementations of some interfaces that should contain conventions.
Also, separation of concern for engine host has been improved, and a simple base implementation has been added. Tools should use this (or any subclasses provided) to implement their own EngineHost. This includes Google, which will use the FileSystem based one, and the Workbench which will use the Registry one.
packages/schematics/package.json
Outdated
"blueprints", | ||
"code generation", | ||
"schematics", | ||
"Angular SDK" |
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.
sdk -> devkit
Angular SDK -> Angular DevKit
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.
Done.
packages/schematics/package.json
Outdated
], | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/angular/sdk.git" |
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.
repo name/url has changed
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.
Done.
packages/schematics/package.json
Outdated
"bugs": { | ||
"url": "https://github.com/angular/sdk/issues" | ||
}, | ||
"homepage": "https://github.com/angular/sdk", |
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.
angular/devkit again
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.
Done.
export * from './utility/path'; | ||
|
||
|
||
export interface TreeConstructor { |
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.
I find it odd to have these tree definitions inside index.
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.
Needs to be done where it's exported, but not in the interface definition (there's a circular dependency).
import {BaseException} from '../exception/exception'; | ||
|
||
import {Observable} from 'rxjs/Observable'; | ||
import 'rxjs/add/observable/fromPromise'; |
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.
fromPromise is not used
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.
Removed.
c: 2 | ||
}); | ||
}); | ||
}); |
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.
Newline
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.
Done. Added linting too.
packages/schematics_cli/package.json
Outdated
], | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/angular/sdk.git" |
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.
devkit, not sdk
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.
Done.
packages/schematics_cli/package.json
Outdated
"author": "Angular Authors", | ||
"license": "MIT", | ||
"bugs": { | ||
"url": "https://github.com/angular/sdk/issues" |
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.
devkit, not sdk
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.
Done.
packages/schematics_cli/package.json
Outdated
"url": "https://github.com/angular/sdk/issues" | ||
}, | ||
"schematics": "./schematics/collection.json", | ||
"homepage": "https://github.com/angular/sdk", |
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.
devkit, not sdk
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.
Done.
Also added linting and fix those and a travis configuration.
*/ | ||
import {FileSystemSink} from './filesystem'; | ||
|
||
import {Observable} from 'rxjs'; |
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.
prefer import {Observable} from 'rxjs/Observable';
@wKoza agreed, want to submit a PR for that one and the import of |
No description provided.