-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
[Windows] Make sure that FileSystem.Current.AppDataDirectory
folder exists in unpackaged
#23265
[Windows] Make sure that FileSystem.Current.AppDataDirectory
folder exists in unpackaged
#23265
Conversation
Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
@@ -8,7 +8,7 @@ namespace Microsoft.Maui.Essentials.DeviceTests | |||
[Category("FileSystem")] | |||
public class FileSystem_Tests | |||
{ | |||
const string bundleFileContents = "This file was in the app bundle."; | |||
const string BundleFileContents = "This file was in the app bundle."; |
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.
Just code style changes in this file.
|
||
if (!File.Exists(_platformAppDataDirectory)) | ||
{ | ||
Directory.CreateDirectory(_platformAppDataDirectory); |
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.
It is not great that this method can throw an exception in the constructor.
=> AppInfoUtils.IsPackagedApp | ||
? ApplicationData.Current.LocalFolder.Path | ||
: Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), AppSpecificPath, "Data"); | ||
string PlatformAppDataDirectory => _platformAppDataDirectory; |
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.
Maybe instead of moving the logic the constructor, it might be better to do all the checks and creating in this property? So this property will throw vs the ctor.
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.
But then you would have some perf penalty on every access. Would that be fine or not?
Ah you probably meant using Lazy. That might be a viable way indeed.
@mattleibow Does this look good to you? |
/azp run |
Very nice, thanks. |
Azure Pipelines successfully started running 3 pipeline(s). |
Description of Change
I'm not too sure if the PR is correct or not as I haven't figured out how to run Windows tests locally.
Anyway, #22231 mentions
C:\Users\davidortinau\AppData\Roaming\User Name\com.simplyprofound.sentencestudio\Data\
and thatData
folder comes from this place.Issues Fixed
Fixes #22231