feat(command): adding the --app command option by sumitarora · Pull Request #4754 · angular/angular-cli

sumitarora · 2017-02-16T17:20:43Z

Added option for ng serve --app <app_name> to commands build, serve, generate, eject, i18n

hansl

First comments.

hansl · 2017-02-17T18:27:37Z

@@ -0,0 +1,11 @@
+export function getAppFromConfig(apps: any, name: String) {


We have typing for the configuration.

There is interface for whole config not apps part.

hansl · 2017-02-17T18:28:35Z


-module.exports = function dynamicPathParser(project, entityName) {
+module.exports = function dynamicPathParser(project, entityName, appConfig) {
+


Remove empty line.

hansl · 2017-02-17T18:31:11Z

@@ -1,3 +1,5 @@
+import * as appUtils from '../../utilities/app-utils';


Instead of import * as, just import the function you need. This is a TypeScript file, not a CommonJS one.

Here and everywhere.

hansl · 2017-02-17T18:32:52Z


-export default function findParentModule(project: any, currentDir: string): string {
-  const sourceRoot = path.join(project.root, project.ngConfig.apps[0].root, 'app');
+export default function findParentModule


Put as many arguments on the same line as function declaration, then indent 4 (or left align with first argument) the rest.

hansl · 2017-02-17T18:33:20Z

    } else {
      try {
-        this.pathToModule = findParentModule(this.project, this.dynamicPath.dir);
+        this.pathToModule = findParentModule


Put as many arguments on the same line as function declaration, then indent 4 (or left align with first argument) the rest.

Here and everywhere.

filipesilva · 2017-02-21T15:58:21Z


  availableOptions: baseBuildCommandOptions.concat([
-    { name: 'stats-json', type: Boolean, default: false }
+    { name: 'watch', type: Boolean, default: false, aliases: ['w'] },


Watch is now part of the base options, I think you left it here as part of a bad rebase.

filipesilva · 2017-02-21T15:59:07Z

    baseServeCommandOptions.concat([
      { name: 'config', type: String, aliases: ['c'] },
      { name: 'specs', type: Array, default: [], aliases: ['sp'] },
+      { name: 'app', type: String, aliases: ['a'] },


Add it to baseBuildCommandOptions instead, and build/serve/e2e will all use it.

filipesilva · 2017-02-21T16:05:29Z

@@ -0,0 +1,11 @@
+export function getAppFromConfig(apps: any, name: String) {


This looks like a pretty good candidate for a static method on models/config. What do you think?

it's used by karma.js too so it's better as independent util function

I changed the whole testing config setup in the last day so it should be much more like the others.

filipesilva · 2017-02-21T16:09:48Z

      description: 'Specifies if a spec file is generated.'
+    },
+    {
+      name: 'app',


This gets repeated a lot. It's probably better to make some kind of base BaseAppOptions+AppOptions similar to baseBuildCommandOptions+BuildOptions (or a better abstraction if you can find it).

That way if we need to change it, or add more app options in the future, it's only in one place.

sure we should do it as a refactoring task as it needs to be done in multiple commands

Let's do it separately.

If we're doing that we should also be doing the same thing with things like flat and spec too

googlebot · 2017-02-21T22:58:57Z

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.

hansl

1 more nit.

hansl · 2017-02-22T00:15:23Z

@@ -0,0 +1,13 @@
+import {CliConfig as CliConfigInterface} from '../lib/config/schema';
+
+export function getAppFromConfig(apps: CliConfigInterface['apps'], name: String) {


as a fallback, if the name can be used as a number (using name.match(/^[0-9]+$/)), use parseInt(name, 10) to get it. This way users can do ng build --app=1 and get the first app.

googlebot · 2017-02-22T00:32:45Z

CLAs look good, thanks!

filipesilva · 2017-02-22T15:17:24Z

 ];

 export interface BuildTaskOptions extends BuildOptions {
+  watch?: boolean;


This line is not needed, watch is in BuildOptions;

filipesilva · 2017-02-22T15:19:41Z


 export interface BuildTaskOptions extends BuildOptions {
+  watch?: boolean;
+  app?: String;


Add this to https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/models/build-options.ts instead

filipesilva · 2017-02-22T15:20:54Z

@@ -0,0 +1,11 @@
+export function getAppFromConfig(apps: any, name: String) {


I changed the whole testing config setup in the last day so it should be much more like the others.

filipesilva · 2017-02-22T15:24:35Z


 const init: any = (config: any) => {
-  const appConfig = CliConfig.fromProject().config.apps[0];
+  // load Angular CLI config


This isn't needed anymore, I think you left it here as part of the rebase.

filipesilva · 2017-02-22T15:26:29Z

  sslCert?: string;
  open?: boolean;
  hmr?: boolean;
+  app?: string;


Remove app from here and instead add it to https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/models/build-options.ts.

mautrok · 2017-04-06T09:28:49Z

I try to do ng build --app 1 where 1 is the index of a configuration inside the "apps" field, but it seesms not work, what do i do wrong?

"apps": [
{
"root": "src",
"outDir": "dist1",
"assets": [
"fonts",
"images",
"assets",
"favicon.ico"
],
"index": "index.html",
"main": "main.ts",
"test": "test.ts",
"tsconfig": "tsconfig.json",
"prefix": "app",
"mobile": false,
"styles": [
"paper.css",
"styles.css",
"style_1.css"
],
"scripts": ["../node_modules/chart.js/dist/Chart.bundle.min.js"],
"environments": {
"source": "environments/environment.ts",
"dev": "environments/environment.ts",
"prod": "environments/environment.prod.ts"
}
},
{
"root": "src",
"outDir": "dist2",
"assets": [
"fonts",
"images",
"assets",
"favicon.ico"
],
"index": "index.html",
"main": "main.ts",
"test": "test.ts",
"tsconfig": "tsconfig.json",
"prefix": "app",
"mobile": false,
"styles": [
"paper.css",
"styles.css",
"style_2.css"
],
"scripts": ["../node_modules/chart.js/dist/Chart.bundle.min.js"],
"environments": {
"source": "environments/environment.ts",
"dev": "environments/environment.ts",
"prod": "environments/environment.prod.ts"
}
}

angular-automatic-lock-bot · 2019-09-11T23:42:31Z

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.}

sumitarora added the state: WIP label Feb 16, 2017

sumitarora requested a review from hansl February 16, 2017 17:20

googlebot added the cla: yes label Feb 16, 2017

sumitarora force-pushed the feat-app branch 9 times, most recently from b246573 to 6cb983b Compare February 17, 2017 18:21

hansl suggested changes Feb 17, 2017

View reviewed changes

sumitarora force-pushed the feat-app branch 4 times, most recently from 7e884e9 to 4fed19c Compare February 21, 2017 15:08

sumitarora removed the state: WIP label Feb 21, 2017

sumitarora force-pushed the feat-app branch 3 times, most recently from 2422388 to 428957f Compare February 21, 2017 16:00

filipesilva reviewed Feb 21, 2017

View reviewed changes

sumitarora force-pushed the feat-app branch 3 times, most recently from dacbfd5 to 0487edb Compare February 21, 2017 22:58

googlebot added cla: no and removed cla: yes labels Feb 21, 2017

sumitarora force-pushed the feat-app branch from 0487edb to c4aff04 Compare February 21, 2017 23:16

hansl suggested changes Feb 22, 2017

View reviewed changes

sumitarora force-pushed the feat-app branch 2 times, most recently from d1c30e1 to 55ab668 Compare February 22, 2017 00:32

googlebot added cla: yes and removed cla: no labels Feb 22, 2017

filipesilva mentioned this pull request Feb 22, 2017

4506 remove app hardcode #4610

Closed

sumitarora force-pushed the feat-app branch 2 times, most recently from ce07732 to b213d72 Compare February 22, 2017 14:41

filipesilva reviewed Feb 22, 2017

View reviewed changes

filipesilva suggested changes Feb 22, 2017

View reviewed changes

sumitarora force-pushed the feat-app branch 3 times, most recently from 7a31214 to 839b9ce Compare February 22, 2017 17:07

filipesilva approved these changes Feb 22, 2017

View reviewed changes

sumitarora force-pushed the feat-app branch from 839b9ce to 452f827 Compare February 22, 2017 18:23

hansl approved these changes Feb 22, 2017

View reviewed changes

feat(command): adding the --app command option

bf55d24

sumitarora force-pushed the feat-app branch from 452f827 to bf55d24 Compare February 22, 2017 18:44

hansl merged commit ade2236 into angular:master Feb 22, 2017

jschwarty mentioned this pull request Feb 24, 2017

Angular cli beta.31+ breaks playground SoCreate/angular-playground#11

Closed

This was referenced Mar 9, 2017

Allow user to specify angular-cli.json file to be used #4495

Closed

I don't want to call my 'app', 'app'. #4506

Closed

sumitarora deleted the feat-app branch March 24, 2017 16:35

sumitarora mentioned this pull request Mar 27, 2017

feat(serve): Add -main flag for entry point #3940

Closed

asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017

feat(@angular/cli): adding the --app command option (angular#4754)

fb37dbe

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 11, 2019

		@@ -0,0 +1,11 @@
		export function getAppFromConfig(apps: any, name: String) {


		module.exports = function dynamicPathParser(project, entityName) {
		module.exports = function dynamicPathParser(project, entityName, appConfig) {

		@@ -1,3 +1,5 @@
		import * as appUtils from '../../utilities/app-utils';

		@@ -0,0 +1,13 @@
		import {CliConfig as CliConfigInterface} from '../lib/config/schema';

		export function getAppFromConfig(apps: CliConfigInterface['apps'], name: String) {

Conversation

sumitarora commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hansl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sumitarora Feb 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipesilva Feb 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

googlebot commented Feb 21, 2017

Uh oh!

hansl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

googlebot commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mautrok commented Apr 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sumitarora commented Feb 16, 2017 •

edited

Loading

sumitarora Feb 21, 2017 •

edited

Loading

filipesilva Feb 21, 2017 •

edited

Loading

mautrok commented Apr 6, 2017 •

edited

Loading