Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Updates to configurations view to work with subdirectories #1956

Open
wants to merge 2 commits into
base: sagerb-show-projectDir-when-selecting-entrypoints
Choose a base branch
from
Open

Updates to configurations view to work with subdirectories #1956

wants to merge 2 commits into from
+18 −5

Conversation

sagerb
Copy link
Collaborator

@sagerb sagerb commented Jul 12, 2024 •
edited
Loading

Intent

Resolves #1951

  • move progress indicator to configurations view
  • update clone initial name for projectDir
  • add progress indicator for children tree population (relies upon sometimes slow API)

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

Automated Tests

Directions for Reviewers

  • Verify operations work for configurations view
  • Verify that when creating a new configuration when VSCode has opened a folder containing a lot of entrypoints, that the progress indicator is shown within the Configurations view (vs. down below in the bottom status line).
  • Verify that when opening very large directory tree and switching over to publisher, that configurations window when expanded shows progress indicator until it is able to load.

Checklist

@sagerb sagerb self-assigned this Jul 12, 2024
dotNomad
dotNomad approved these changes Jul 15, 2024
View reviewed changes
Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic. Just had one syntax question

extensions/vscode/src/views/configurations.ts
Comment on lines +88 to +90
async () => {
return getAllPromise;
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just pass the getAllPromise here rather than wrapping it in another function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotNomad dotNomad dotNomad approved these changes

@kgartland-rstudio kgartland-rstudio Awaiting requested review from kgartland-rstudio kgartland-rstudio is a code owner

@mmarchetti mmarchetti Awaiting requested review from mmarchetti mmarchetti is a code owner

@tdstein tdstein Awaiting requested review from tdstein tdstein is a code owner

Assignees

@sagerb sagerb

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sagerb @dotNomad