-
Notifications
You must be signed in to change notification settings - Fork 161
feat: Deploy pre-existing package with -p
CLI option
#205
Conversation
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.
have some questions
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 good!
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 style nit
|
||
const defaultArtifact = path.join(".serverless", `${sls.service.getServiceName()}.zip`); | ||
|
||
expect((FunctionAppService.prototype as any).sendFile).toBeCalledWith({ |
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: I would extract the toBeCalledWith
arguments out just so it's easier to read
} | ||
}, defaultArtifact); | ||
const expectedArtifactName = service.getDeploymentName().replace("rg-deployment", "artifact"); | ||
expect((AzureBlobStorageService.prototype as any).uploadFile).toBeCalledWith( |
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: extract AzureBlobStorageService.prototype as any
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.
LGTM
- [x] Allow `-p` or `--package` param to specify package to deploy - [x] Log in before deployment rather than packaging - [x] Don't run deployment if local package does not exist Resolves [AB#500]
-p
or--package
param to specify package to deployResolves [AB#500]