Skip to content

feat(@angular/cli): making app root configurable - #6079

Closed
sumitarora wants to merge 2 commits into
angular:masterfrom
sumitarora:feat-approot-config
Closed

feat(@angular/cli): making app root configurable#6079
sumitarora wants to merge 2 commits into
angular:masterfrom
sumitarora:feat-approot-config

Conversation

@sumitarora

@sumitarora sumitarora commented Apr 25, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes: #5927

hansl
hansl suggested changes Apr 25, 2017
View reviewed changes
Comment thread packages/@angular/cli/blueprints/component/index.ts Outdated
} else {
try {
this.pathToModule = findParentModule(this.project.root, appConfig.root, this.generatePath);
this.pathToModule = findParentModule

@hansl hansl Apr 25, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parentheses on same line as function. Same everywhere.

@sumitarora sumitarora Apr 25, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@hansl done

Brocco
Brocco suggested changes Apr 25, 2017
View reviewed changes

@Brocco Brocco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few things:
If you're updating angular-cli.json you should also update the schema. The default will be needed for backwards compatability.
Tests?

@sumitarora
sumitarora force-pushed the feat-approot-config branch from 0f79e8f to f34777f Compare April 25, 2017 19:17
@sumitarora

sumitarora commented Apr 25, 2017

Copy link
Copy Markdown
Contributor Author

@Brocco Added Test 😄

@clydin

clydin commented Apr 25, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Looking at the blueprint, what is currently not configurable is the main ng module name (I.e, the bootstrapped module) and component name. A module's containing directory is also the name of the module so allowing configurability should allow the underlying issue's use case.
The "root" setting really is the app root as a large amount of app specific files are located there (index.html, main.ts, etc.).

@hansl
hansl changed the base branch from master to 1.1.x April 25, 2017 23:50
@Brocco

Brocco commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

Can you please rebase & repush this PR? There's a lot of noise in the diffs to review.

@sumitarora
sumitarora force-pushed the feat-approot-config branch from abecf30 to c50a689 Compare April 26, 2017 13:20
@googlebot

googlebot commented Apr 26, 2017

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@sumitarora
sumitarora force-pushed the feat-approot-config branch from c50a689 to 586b6be Compare April 26, 2017 18:16
@googlebot

googlebot commented Apr 26, 2017

Copy link
Copy Markdown

CLAs look good, thanks!

@sumitarora

sumitarora commented Apr 26, 2017

Copy link
Copy Markdown
Contributor Author

@hansl @Brocco Updated and rebased 😄

@hansl
hansl changed the base branch from 1.1.x to master May 4, 2017 00:31
@hansl

hansl commented May 4, 2017

Copy link
Copy Markdown
Contributor

Please rebase.

@sumitarora
sumitarora force-pushed the feat-approot-config branch from 586b6be to af59849 Compare May 4, 2017 01:14
@googlebot

googlebot commented May 4, 2017

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added cla: no and removed cla: yes labels May 4, 2017
@sumitarora
sumitarora force-pushed the feat-approot-config branch from af59849 to 9dfecb2 Compare May 4, 2017 13:36
@sumitarora sumitarora removed the cla: no label May 4, 2017
@sumitarora
sumitarora force-pushed the feat-approot-config branch from 9dfecb2 to 08e7c97 Compare May 5, 2017 14:05
@googlebot googlebot added cla: no and removed cla: yes labels May 5, 2017
@sumitarora
sumitarora force-pushed the feat-approot-config branch from 08e7c97 to a64c57d Compare May 24, 2017 15:16
@sumitarora
sumitarora force-pushed the feat-approot-config branch from a64c57d to cc5c7bb Compare May 26, 2017 13:17
@hansl

hansl commented Sep 27, 2017

Copy link
Copy Markdown
Contributor

Closing every Blueprints related issue and PR.

@hansl hansl closed this Sep 27, 2017
This was referenced Oct 5, 2017
Closed
Closed
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 12, 2019

Copy link
Copy Markdown

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 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@filipesilva filipesilva Awaiting requested review from filipesilva
2 more reviewers
@hansl hansl hansl requested changes
@Brocco Brocco Brocco requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Brocco Brocco

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ng generate <blueprint> assumes top level component is named "app"

5 participants

@sumitarora @clydin @Brocco @googlebot @hansl