Skip to content
This repository was archived by the owner on Jun 5, 2025. It is now read-only.

Data storage (i.e. extension and registered root path) for version control extensions inside Dev Home #2984

Merged
ssparach merged 4 commits into from
Jun 3, 2024
Merged

Data storage (i.e. extension and registered root path) for version control extensions inside Dev Home #2984

ssparach merged 4 commits into from
Jun 3, 2024

Conversation

@ssparach
Copy link
Contributor

@ssparach ssparach commented May 22, 2024 •
edited
Loading

Summary of the pull request

This PR contains the changes required to store and retrieve data from a json file required for source control integration.

References and relevant issues

Detailed description of the pull request / Additional comments

The code changes included are:

  • Read/Write extension, root path pairs (JSON file)
  • Temporary UI (the UI currently outlined in this PR will be removed and is in place currently to provide basic add functionality). The new UI will be in accordance to design specifications and is being tracked.
  • Unit Test

Validation steps performed

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

dhoehna
dhoehna reviewed May 22, 2024
View reviewed changes
tools/Customization/DevHome.Customization/Views/FileExplorerPage.xaml.cs Outdated
if (!Path.IsPathFullyQualified(rootPath))
{
// Error: Path is not fully qualified or maybe a UNC path.
RootPathErrorBar.IsOpen = true;
Copy link
Contributor

@dhoehna dhoehna May 22, 2024

Choose a reason for hiding this comment

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

NIT: Change the error message based on what is being checked. Give the user the exact reason why the path isn't valid.

Copy link
Member

@DefaultRyan DefaultRyan May 23, 2024

Choose a reason for hiding this comment

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

NIT: Change the error message based on what is being checked. Give the user the exact reason why the path isn't valid.

+1 to this. We may even consider having some different error/exception types for path checking.

Once interesting example is that repositories mounted on WSL look sorta like UNC paths, but we should try to detect that case separate from UNC paths. Also with WSL repos, their file ownership looks off from Windows' point of view, so Git will raise specific errors about those cases. Rather than outright failing, we should offer a suggestion to the user to help them enable Git to work with those WSL repos, aka git config --global --add safe.directory <path to repo>

Copy link
Contributor Author

@ssparach ssparach May 31, 2024

Choose a reason for hiding this comment

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

Sounds great! Currently, the UI in place is temporary. I will incorporate this feedback and gain more clarity around the design for surfacing user facing error messages in the expected UX.

krschau
krschau reviewed May 22, 2024
View reviewed changes
@ssparach ssparach merged commit 32e55b0 into feature/fileexplorer-sourcecontrol-integration Jun 3, 2024
@krschau krschau added this to the Dev Home 0.15 milestone Jun 3, 2024
ssparach added a commit that referenced this pull request Jul 31, 2024
@ssparach
Data storage (i.e. extension and registered root path) for version co…
a85702b 
…ntrol extensions inside Dev Home (#2984)

* changes to write/read data (extension information + root path) from json

* address PR comments

* revert line endings

* address PR comments
@ssparach ssparach deleted the user/ssparach/datastorage branch August 2, 2024 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DefaultRyan DefaultRyan Awaiting requested review from DefaultRyan

@vineeththomasalex vineeththomasalex Awaiting requested review from vineeththomasalex

@jamespik jamespik Awaiting requested review from jamespik

2 more reviewers

@krschau krschau krschau left review comments

@dhoehna dhoehna dhoehna approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Dev Home 0.15

Development

Successfully merging this pull request may close these issues.

5 participants

@ssparach @dhoehna @DefaultRyan @krschau