Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Ng schematics service #26

Merged
merged 1 commit into from
Jul 13, 2017
Merged

Conversation

jschwarty
Copy link
Contributor

Have a TODO in there for the module name option as per discussion with @Brocco. Also, the package-lock.json file is in here. Not sure what the plan is to handle that as it changes from dev to dev. Let me know if I need to do anything on that.

@Brocco
Copy link
Contributor

Brocco commented Jul 12, 2017

@jschwarty PR #29 contains the updates for module resolutions.

}

let modulePath;
if (options.module) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is redundant (see line 33)
Once that's removed modulePath can be a const instead of being let and set on line 42.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like this?

    if (!options.module) {
      return host;
    }

    const modulePath = options.module;
    if (!host.exists(options.module)) {
      throw new Error(`Module specified (${options.module}) does not exist.`);
    }

@@ -12,10 +12,14 @@
"@ngtools/logger": {
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be no changes to package-lock.json in this PR either

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a bad merge. just reset it.

@jschwarty jschwarty force-pushed the ng-schematics-service branch from 8fb84b5 to 021a02e Compare July 13, 2017 17:52
template,
url,
} from '@angular-devkit/schematics';
import * as stringUtils from '../strings';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please merge all import groups together (this is different than the CLI repo).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just no empty line here between import lines, correct?

@jschwarty jschwarty force-pushed the ng-schematics-service branch from 021a02e to 2a089e2 Compare July 13, 2017 18:58
@Brocco Brocco merged commit 8627da4 into angular:master Jul 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants