Swap entire project to command palette experience #61
Conversation
* add draft dockerfile command * add file * fix context stuff * add wizard * add wizard * add draft cmd * add open files step * add save state step * add validation * add run draft dockerfile experience * add open docs link * add validation test * refactor port to common * add ns * fix application name guess * add image validation * add choose image from acr options * add override check * add draft execute * add open files * remove hardcoded audience * refactor logic into create client fn * default to local folder * refactor * swap to async opts * add extra comment * remove empty file * wrap error message * swap to fs path * fix back bug * revert version * swap to fs path * remove other commands * fix typos
There was a problem hiding this comment.
☕️💡🙏 Thanks @qpetraroia for FYI, added very high level quick eyeball comments. Rest folks who are more involved might be able to add more into the PR, thank you all.
| @@ -27,28 +25,20 @@ | |||
| "properties": { | |||
| "aks.draft.releaseTag": { | |||
| "type": "string", | |||
| "default": "v0.0.22", | |||
There was a problem hiding this comment.
💡 Package version update could be part of release PR not the code PR.
There was a problem hiding this comment.
Thank you for review!
Aligned on the package version update being part of the release PR. There's not package version update happening in this PR currently.
The quoted text is the pinned Draft version tag which needs to be bumped in this PR since we are relying on features from new Draft versions.
| "group": "draftnavigation@2" | ||
| }, |
There was a problem hiding this comment.
💡 Thank you @qpetraroia for quick conversation, since this is removing whole lot of existing functionality, shall we open a tracking issue against repo and Pin it as a breaking change for anyone or 1000 users using this extension in current state. Just an Idea I thought I will FYI.
There was a problem hiding this comment.
Thanks for the review @Tatsinnit, yes that is something we will be doing! Documentation and a pinned issue will be created/merged before this change goes live
| createNamespace(name: string): Promise<Errorable<void>>; | ||
| applyManifests(manifestPaths: string[]): Promise<Errorable<string>>; | ||
| applyKustomize(kustomizationDirectory: string): Promise<Errorable<string>>; | ||
| installHelm(chartDirectory: string): Promise<Errorable<string>>; |
There was a problem hiding this comment.
💡 just fyi - please do check, If we have taken dependency on Kubernetes extension I think helm install comes with it, so installing helm at this extension might not be needed. Thanks.
There was a problem hiding this comment.
Aligned on that! We do take dependency on the Kubernetes extension and we do get the Helm install from them. You are completely correct.
|
Hold off on merge until docs and pinned issue is created |
Description
Swaps the entire project to the command palette experience.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
.vsix was tested and checked off by Ahmed and Quentin across a number of systems and scenarios. Unit tests for things that are possible.
Checklist: