More caches added, other improvements #2551
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,12 +12,17 @@ | |
| using DevHome.Customization.Helpers; | ||
| using DevHome.Customization.ViewModels.DevDriveInsights; | ||
| using DevHome.Customization.Views; | ||
| using Microsoft.Internal.Windows.DevHome.Helpers; | ||
| using Serilog; | ||
|
|
||
| namespace DevHome.Customization.ViewModels; | ||
|
|
||
| public partial class DevDriveInsightsViewModel : ObservableObject | ||
| { | ||
| private readonly ShellSettings _shellSettings; | ||
|
|
||
| public ObservableCollection<Breadcrumb> Breadcrumbs { get; } | ||
|
|
||
| public ObservableCollection<DevDriveCardViewModel> DevDriveCardCollection { get; private set; } = new(); | ||
|
|
||
| public ObservableCollection<DevDriveOptimizerCardViewModel> DevDriveOptimizerCardCollection { get; private set; } = new(); | ||
|
|
@@ -48,12 +53,31 @@ public partial class DevDriveInsightsViewModel : ObservableObject | |
|
|
||
| private IEnumerable<IDevDrive> ExistingDevDrives { get; set; } = Enumerable.Empty<IDevDrive>(); | ||
|
|
||
| private static readonly string _appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData); | ||
|
There was a problem hiding this comment. @krschau I know StyleCop changed recently. I remember StyleCop wanting all There was a problem hiding this comment. I'm still seeing this error if I don't follow the rule locally in the Dashboard project. All projects should use the same stylecop.json and globalsuppressions.cs. The static keyword seems to be making the error go away? Nothing in the doc for this rule explains what's happening. https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1214.md |
||
|
|
||
| private static readonly string _localAppDataPath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); | ||
|
|
||
| private static readonly string _userProfilePath = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); | ||
|
|
||
| private const string ExampleDevDriveDirStr = "D:"; | ||
|
There was a problem hiding this comment. This'll bit a bit awkward if the example shows D: but the user is already ising their D: drive. #Resolved There was a problem hiding this comment. This is a valid point. Let me switch to an existing dev drive. |
||
|
|
||
| private const string PackagesStr = "packages"; | ||
|
|
||
| private const string CacheStr = "cache"; | ||
|
|
||
| private const string ArchivesStr = "archives"; | ||
|
|
||
| public DevDriveInsightsViewModel(IDevDriveManager devDriveManager, OptimizeDevDriveDialogViewModelFactory optimizeDevDriveDialogViewModelFactory) | ||
| { | ||
| _shellSettings = new ShellSettings(); | ||
|
|
||
| var stringResource = new StringResource("DevHome.Customization.pri", "DevHome.Customization/Resources"); | ||
| Breadcrumbs = | ||
| [ | ||
| new(stringResource.GetLocalized("MainPage_Header"), typeof(MainPageViewModel).FullName!), | ||
| new(stringResource.GetLocalized("DevDriveInsights_Header"), typeof(DevDriveInsightsViewModel).FullName!) | ||
| ]; | ||
|
|
||
| _optimizeDevDriveDialogViewModelFactory = optimizeDevDriveDialogViewModelFactory; | ||
| DevDriveManagerObj = devDriveManager; | ||
| } | ||
|
|
@@ -237,17 +261,60 @@ public void UpdateListViewModelList() | |
| EnvironmentVariable = "PIP_CACHE_DIR", | ||
| CacheDirectory = new List<string> | ||
| { | ||
| Path.Join(_localAppDataPath, "pip", CacheStr), | ||
| Path.Join(_localAppDataPath, PackagesStr, "PythonSoftwareFoundation.Python"), | ||
| }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "pip", CacheStr), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "NuGet cache (dotnet)", | ||
| EnvironmentVariable = "NUGET_PACKAGES", | ||
| CacheDirectory = new List<string> { Path.Join(_userProfilePath, ".nuget", PackagesStr) }, | ||
dhoehna marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "NuGet", CacheStr), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "Npm cache (NodeJS)", | ||
|
There was a problem hiding this comment. Also, why are these values hard coded? Shouldn't we not be doing this so DevHome can stay generic? #ByDesign There was a problem hiding this comment. Currently for V1, we are only looking to optimize for these caches- https://learn.microsoft.com/en-us/windows/dev-drive/#storing-package-cache-on-dev-drive. In future we are looking to extend this to allow the user to input the existing cache location and env variable so that it works for any package cache. But that design is not finalized yet. There was a problem hiding this comment. Does this need localization? There was a problem hiding this comment.
I don't think so because these are product names. |
||
| EnvironmentVariable = "NPM_CONFIG_CACHE", | ||
| CacheDirectory = new List<string> | ||
| { | ||
| Path.Join(_appDataPath, "npm-cache"), | ||
| Path.Join(_localAppDataPath, "npm-cache"), | ||
| }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "npm"), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "Vcpkg cache", | ||
| EnvironmentVariable = "VCPKG_DEFAULT_BINARY_CACHE", | ||
| CacheDirectory = new List<string> | ||
| { | ||
| Path.Join(_appDataPath, "vcpkg", ArchivesStr), | ||
| Path.Join(_localAppDataPath, "vcpkg", ArchivesStr), | ||
| }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "vcpkg"), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "Cargo cache (Rust)", | ||
| EnvironmentVariable = "CARGO_HOME", | ||
| CacheDirectory = new List<string> { Path.Join(_userProfilePath, ".cargo") }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "cargo"), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "Maven cache (Java)", | ||
| EnvironmentVariable = "MAVEN_OPTS", | ||
| CacheDirectory = new List<string> { Path.Join(_userProfilePath, ".m2") }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "m2"), | ||
| }, | ||
| new DevDriveCacheData | ||
| { | ||
| CacheName = "Gradle cache (Java)", | ||
| EnvironmentVariable = "GRADLE_USER_HOME", | ||
| CacheDirectory = new List<string> { Path.Join(_userProfilePath, ".gradle") }, | ||
| ExampleDirectory = Path.Join(ExampleDevDriveDirStr, PackagesStr, "gradle"), | ||
| } | ||
| ]; | ||
|
|
||
|
|
@@ -261,13 +328,13 @@ public void UpdateListViewModelList() | |
| } | ||
| else | ||
| { | ||
| var subDirectories = Directory.GetDirectories(Path.Join(_localAppDataPath, PackagesStr), "*", SearchOption.TopDirectoryOnly); | ||
| var matchingSubdirectory = subDirectories.FirstOrDefault(subdir => subdir.StartsWith(cacheDirectory, StringComparison.OrdinalIgnoreCase)); | ||
| if (Directory.Exists(matchingSubdirectory)) | ||
| { | ||
| if (matchingSubdirectory.Contains("PythonSoftwareFoundation")) | ||
| { | ||
| return Path.Join(matchingSubdirectory, "LocalCache", "Local", "pip", CacheStr); | ||
| } | ||
|
|
||
| return matchingSubdirectory; | ||
|
|
@@ -307,12 +374,19 @@ public void UpdateOptimizerListViewModelList() | |
| continue; | ||
| } | ||
|
|
||
| List<string> existingDevDriveLetters = new List<string>(); | ||
|
There was a problem hiding this comment. NIT: |
||
| foreach (var existingDevDrive in ExistingDevDrives) | ||
| { | ||
| existingDevDriveLetters.Add(existingDevDrive.DriveLetter.ToString()); | ||
| } | ||
|
|
||
| var card = new DevDriveOptimizerCardViewModel( | ||
| _optimizeDevDriveDialogViewModelFactory, | ||
| cache.CacheName!, | ||
| existingCacheLocation, | ||
| cache.ExampleDirectory!, // example location on dev drive to move cache to | ||
| cache.EnvironmentVariable!, // environmentVariableToBeSet | ||
| existingDevDriveLetters); | ||
| DevDriveOptimizerCardCollection.Add(card); | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,10 @@ | |
| x:Class="DevHome.Customization.Views.DevDriveInsightsPage" | ||
| xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" | ||
| xmlns:behaviors="using:DevHome.Common.Behaviors" | ||
| xmlns:views="using:DevHome.Customization.Views" | ||
| behaviors:NavigationViewHeaderBehavior.HeaderTemplate="{StaticResource BreadcrumbBarDataTemplate}" | ||
| behaviors:NavigationViewHeaderBehavior.HeaderContext="{x:Bind ViewModel}"> | ||
|
|
||
| <Grid MaxWidth="{ThemeResource MaxPageContentWidth}" Margin="{ThemeResource ContentPageMargin}"> | ||
| <Grid.RowDefinitions> | ||
|
There was a problem hiding this comment. You don't need the RowDefinitions (or to set the Grid.Row on the ScrollView) since there's only one row. |
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this doing? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was needed to pass parameters to the Dialog and from the caller UX and remove the DependencyProperties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. Let me a bit more clear. I can't parse what this code is doing. I've never seen
=>being used twice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also define a ViewModel factory this way in the Dashboard, I think I got the syntax from somewhere in SetupFlow. According to a robot,
Clear as mud :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh
two lambda's. Okay. I understand more.
Maybe add some comments, or expand
sptoserviceProvideror use some whitespace to make this layout more clear.