feat: List deployments sub-command for deploy plugin #176
Conversation
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Left some minor feedback, but looks good overall.
| }; | ||
|
|
||
| this.commands = { | ||
| deploy: { |
There was a problem hiding this comment.
Does defining these commands mess with the top level core plugin on the CLI ?
There was a problem hiding this comment.
I was concerned about that, which is why I left out the lifecycleEvents (I defined them at first, which caused deploy to be triggered twice). I set the plugin to log this.commands to see what the object looks like by default, and itame back undefined. In my manual testing, the deployment still works fine. Feel free to double check me on that though.
| const resourceService = new ResourceService(this.serverless, this.options); | ||
| const deployments = await resourceService.listDeployments(); | ||
| let stringDeployments = "\n\nDeployments"; | ||
| for (const dep of deployments) { |
There was a problem hiding this comment.
If no deployments exist consider adding a line that says "No deployments found for resource group" or similar.
src/services/resourceService.ts
Outdated
| this.resourceClient = new ResourceManagementClient(this.credentials, this.subscriptionId); | ||
| } | ||
|
|
||
| public async listDeployments() { |
There was a problem hiding this comment.
This is more of a "get" operation and think we should rename to getDeployments. The listing/writing is happening in the plugin.
tbarlow12
force-pushed
the
tabarlow/list-deploy
branch
from
35fa9cf to
518144c
Compare
mydiemho
left a comment
mydiemho
left a comment
There was a problem hiding this comment.
looks good. I suggest adding one more tests for empty deployments
| .toBeCalledWith(resourceGroup); | ||
| }); | ||
|
|
||
| it("lists deployments", async () => { |
There was a problem hiding this comment.
another to test when deployments are empty
Resolves [AB#352]