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
feat(@angular/cli): implement deploy command #15105
Conversation
31b1dde
to
f64e55b
Compare
I think including the folks that we worked directly with for the initial
launch would be fair. Otherwise we could say "cloud providers of x
size/revenue/users".
…On Thu, Jul 18, 2019 at 3:22 PM Minko Gechev ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/angular/cli/commands/deploy-impl.ts
<#15105 (comment)>:
> + * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import { ArchitectCommand, ArchitectCommandOptions } from '../models/architect-command';
+import { Arguments } from '../models/interface';
+import { Schema as DeployCommandSchema } from './deploy';
+
+export class DeployCommand extends ArchitectCommand<DeployCommandSchema> {
+ public readonly target = 'deploy';
+ public readonly error = `Cannot find "deploy" target for the specified project.
+
+You can find a package that implements deployment capabilities for your
+favorite platform on npm: https://www.npmjs.com/search?q=ng%20deploy
What would be the recommendation criteria? We don't have first party
deployment builders.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15105?email_source=notifications&email_token=AABIJQEI6QYFMSEYX5N62ODQADUI3A5CNFSM4IEUYW72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB65VXVI#discussion_r305138216>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABIJQCJFNVMJGDIINW5DADQADUI3ANCNFSM4IEUYW7Q>
.
|
8398e08
to
42b8dde
Compare
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.
Just some initial requests about the commits themselves. I don't have access to the design doc yet.
scripts/validate-user-analytics.ts
Outdated
@@ -26,6 +26,7 @@ const metricsTableRe = /<!--METRICS_TABLE_BEGIN-->([\s\S]*)<!--METRICS_TABLE_END | |||
function _exec(command: string, args: string[], opts: { cwd?: string }, logger: logging.Logger) { | |||
const { status, error, stdout } = spawnSync(command, args, { | |||
stdio: ['ignore', 'pipe', 'inherit'], | |||
maxBuffer: 1024 * 1024, |
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 change should be in it's own commit.
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
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.
Yesterday I was looking at a similar problem in e4e04e2 and now I know that 1024 * 1024
is actually the default.
Is this change actually doing something? I guess only if maxBuffer
was being passed as a smaller value in opts
but I don't find maxBuffer
referenced anywhere in scripts.
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.
Looks like a leftover, just removed it.
22a2218
to
ede7321
Compare
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.
Just a couple questions and minor requests, otherwise LGTM.
scripts/validate-user-analytics.ts
Outdated
@@ -26,6 +26,7 @@ const metricsTableRe = /<!--METRICS_TABLE_BEGIN-->([\s\S]*)<!--METRICS_TABLE_END | |||
function _exec(command: string, args: string[], opts: { cwd?: string }, logger: logging.Logger) { | |||
const { status, error, stdout } = spawnSync(command, args, { | |||
stdio: ['ignore', 'pipe', 'inherit'], | |||
maxBuffer: 1024 * 1024, |
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.
Yesterday I was looking at a similar problem in e4e04e2 and now I know that 1024 * 1024
is actually the default.
Is this change actually doing something? I guess only if maxBuffer
was being passed as a smaller value in opts
but I don't find maxBuffer
referenced anywhere in scripts.
{ | ||
"$schema": "http://json-schema.org/schema", | ||
"$id": "ng-cli://commands/deploy.json", | ||
"description": "Invokes the deploy builder for a specified project. If no project is specified, the CLI will invoke the deploy builder for the default project in thw workspace.", |
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.
Nit: thw
-> the
@@ -74,12 +75,13 @@ export abstract class ArchitectCommand< | |||
} | |||
} | |||
|
|||
if (targetProjectNames.length === 0) { | |||
throw new Error(`No projects support the '${this.target}' target.`); | |||
if (!options.help && (targetProjectNames.length === 0)) { |
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.
Why does !options.help
need to added to this condition and the one below it? If I call ng deploy --help
this still looks like the error that shows up.
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.
Since we don't have a default dummy builder, the command is going to always throw when the user invokes ng deploy --help
.
0038e7f
to
c2f50d4
Compare
As discussed today, there's some inconsistent behavior in the CLI in handling the If we improve it, the implementation of |
d68992e
to
ea91049
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Implementation of the
ng deploy
command. Find the design document here.// cc @vikerman @StephenFluin @clydin @alan-agius4 @filipesilva