Basic support for multi-folder deployment creation, selection and deployment #1943
Conversation
…of selected entrypoint.
…ector to facilitate. Updates to saved state as well.
sagerb
changed the base branch from
main
to
sagerb-refactor-underscore-private-names
| export const POSIT_FOLDER = ".posit"; | ||
| export const PUBLISH_FOLDER = ".posit/publish"; | ||
| export const PUBLISH_DEPLOYMENTS_FOLDER = ".posit/publish/deployments"; | ||
| export const POSIT_FOLDER = "**/.posit"; |
There was a problem hiding this comment.
All of our file watcher patterns need now to glob downward into subdirectories.
| @@ -267,7 +267,7 @@ export async function newDeployment( | |||
|
|
|||
| let entryPointListItems: QuickPickItemWithIndex[] = []; | |||
| let inspectionResults: ConfigurationInspectionResult[] = []; | |||
| let contentRecordNames: string[] = []; | |||
| let contentRecordNames = new Map<string, string[]>(); | |||
There was a problem hiding this comment.
switch to a Map here so we can have easy filtering of configurations which are applicable to a projectDir where the entrypoints and deployment is being created.
There was a problem hiding this comment.
I wonder if it would be helpful in other inputs to have a util class for names to add them, use the projectDir to gather them and all that. Something to consider if we repeat the logic elsewhere.
| @@ -367,6 +369,7 @@ export async function newDeployment( | |||
| iconPath: new ThemeIcon("file"), | |||
| label: config.entrypoint, | |||
| description: `(${contentTypeStrings[config.type]})`, | |||
| detail: result.projectDir, | |||
There was a problem hiding this comment.
We needed to indicate the location of the entrypoints, so now I'm using the second line to hold the workingDir
| } | ||
| existingList.push(contentRecord.deploymentName); | ||
| contentRecordNames.set(contentRecord.projectDir, existingList); | ||
| }); |
There was a problem hiding this comment.
Populate the map to be keyed by a projectDir and then contain an array of entrypoints.
| return; | ||
| } | ||
| selectedInspectionResult.configuration.title = state.data.title; | ||
|
|
There was a problem hiding this comment.
same logic above as exists in original, just moved around a bit.
| ); | ||
| if (!existingNames) { | ||
| existingNames = []; | ||
| } |
There was a problem hiding this comment.
easy use of our Map
| @@ -71,15 +72,15 @@ export type RefreshContentRecordDataMsg = AnyHostToWebviewMessage< | |||
| HostToWebviewMessageType.REFRESH_CONTENTRECORD_DATA, | |||
| { | |||
| contentRecords: (ContentRecord | PreContentRecord)[]; | |||
| selectedContentRecordName?: string | null; | |||
| deploymentSelected: DeploymentSelector | null; | |||
There was a problem hiding this comment.
switch to a DeploymentSelector object, which allows us to select the exact combination of deployment and config. It is unique across all locations within a set of directories, since it contains the actual deployment path on disk.
| @@ -89,6 +89,7 @@ export type DeployMsg = AnyWebviewToHostMessage< | |||
| deploymentName: string; | |||
| credentialName: string; | |||
| configurationName: string; | |||
| projectDir: string; | |||
There was a problem hiding this comment.
New parameter needed to deploy!
| }; | ||
|
|
||
| export type DeploymentNames = { | ||
| export type DeploymentSelector = { |
There was a problem hiding this comment.
Replaces use of DeploymentNames (and HomeViewState)
| @@ -354,7 +357,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { | |||
| private async refreshConfigurationData() { | |||
| try { | |||
| const api = await useApi(); | |||
| const response = await api.configurations.getAll(); | |||
| const response = await api.configurations.getAll({ recursive: true }); | |||
There was a problem hiding this comment.
recursively refreshing everytime.. We probably can figure out a more targeted approach, but lets do that in another PR as an optimization.
…cursive-entrypoints
sagerb
requested review from
dotNomad,
kgartland-rstudio,
mmarchetti and
tdstein
as code owners
| selectedConfigurationName?: string | null; | ||
| deploymentSelected: DeploymentSelector | null; |
There was a problem hiding this comment.
Is this no longer optional?
| function updateSelectedConfigurationByName(name: string) { | ||
| function updateSelectedConfigurationByName( | ||
| name: string, | ||
| projectDir?: string, |
There was a problem hiding this comment.
I'm wondering if this should be optional to avoid us changing a selected configuration incorrectly.
| // skip saving the selection if we have not yet | ||
| // selected a content record. This is probably an init | ||
| // sequence. | ||
| if (selectedContentRecord.value) { |
There was a problem hiding this comment.
Bit confused by this change. Wouldn't we want this to update if the deployment selected was deleted for example?
| @@ -430,36 +433,49 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { | |||
| }); | |||
| } | |||
|
|
|||
| private getSelectionState(): HomeViewState { | |||
| const state = this.context.workspaceState.get<HomeViewState>( | |||
| private getSelectionState(): DeploymentSelector | null { | |||
There was a problem hiding this comment.
Shouldn't this still be HomeViewState since that type is DeploymentSelector | null anyway?
| @@ -675,20 +701,16 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { | |||
| } | |||
|
|
|||
| private async propogateDeploymentSelection( | |||
| configurationName?: string, | |||
| deploymentName?: string, | |||
| documentSelector: DeploymentSelector | null, | |||
There was a problem hiding this comment.
nit: this feels like it should be named deploymentSelector
Intent
Resolves #1852
Type of Change
Approach
The changes needed to implement this proved to be pretty much as expected.
Automated Tests
Directions for Reviewers
Unfortunately, there were a number of changes required to maintain functionality once changes were started. It would be best to walk through the individual commits, to understand what is being done.
recursive inspection, create deployment and configuration within dir …
Update watcher patterns to be recursive
Recursive API calls, integrate projectDir and switch to deploymentSel…
Deploy with projectDir parameter (and update deployment tooltip)
Fix codicons path issue (underscore was lost during search/replace)
Checklist