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

refactor(@angular/cli): simplify default arguments in angular-cli.json #4389

Merged
merged 1 commit into from
Feb 16, 2017

Conversation

Brocco
Copy link
Contributor

@Brocco Brocco commented Feb 3, 2017

Fixes #4137

@@ -117,6 +117,10 @@ class SchemaClassBase<T> implements SchemaClass<T> {
/** Get a value from a JSON path. */
$$get(path: string): any {
const node = _getSchemaNodeForPath(this.$$schema(), path);
if (path === 'defaults.component.viewEncapsulation') {
Copy link
Contributor

Choose a reason for hiding this comment

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

tsk tsk

@@ -406,6 +406,7 @@ export abstract class LeafSchemaTreeNode<T> extends SchemaTreeNode<T> {

get() {
if (!this.defined && this._forward) {
console.log('fwd');
Copy link
Contributor

Choose a reason for hiding this comment

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

tsk tsk

@@ -30,7 +30,7 @@ export default Blueprint.extend({

options.spec = options.spec !== undefined ?
options.spec :
this.project.ngConfigObj.get('defaults.spec.class');
this.project.ngConfigObj.get('defaults.class.spec');
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use an alias for these, and a warning if the alias is used.

See here for an example.

@Brocco Brocco force-pushed the style-guide-4137 branch 2 times, most recently from ec17e58 to 80e29b8 Compare February 14, 2017 01:41
@Brocco
Copy link
Contributor Author

Brocco commented Feb 14, 2017

@hansl this PR has been updated addressing the requested changes.

cliConfig.alias('defaults.component.module', 'defaults.spec.module');
cliConfig.alias('defaults.component.pipe', 'defaults.spec.pipe');
cliConfig.alias('defaults.component.service', 'defaults.spec.service');

// If any of them returned true, output a deprecation warning.
if (aliases.some(x => !!x)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This warning should also be updated. Maybe it should be a generic warning? Not sure. I have a PR that will also need to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hansl and I chatted and we decided to leave the remapping of these as silent, as the correct values will still be handled correctly in the config file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll do the same to mine then.

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

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

LGTM.

@hansl hansl merged commit 6febf5c into angular:master Feb 16, 2017
saverett added a commit to saverett/angular-cli that referenced this pull request Feb 24, 2017
…son defaults

Add documentation for a breaking change in angular-cli.json (see angular#4389) that was missed in the changelog

Fix angular#4404
@Brocco Brocco deleted the style-guide-4137 branch March 15, 2017 17:49
asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking: Styleguide Compliance
4 participants