-
Notifications
You must be signed in to change notification settings - Fork 6
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
OSOE-98: Save per-page screenshots to files directly to conserve memory #152
Conversation
This reverts commit a053514.
Lombiq.Tests.UI/Constants/Paths.cs
Outdated
public const string DefaultSetupSnapshotDirectoryPath = "SetupSnapshot"; | ||
public const string TempDirectoryPath = "Temp"; | ||
public const string ScreenshotsDirectoryName = "Screenshots"; |
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.
Two are called *DirectoryPath one is called *DirectoryName. If this is a meaningful distinction, please comment.
By the way if you renamed the class to DirectoryPaths
then you wouldn't need to prefix the variables and you could use nameof
.
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.
How they're used there can be a difference, but let's not have that.
Lombiq.Tests.UI/Constants/Paths.cs
Outdated
public const string TempDirectoryPath = "Temp"; | ||
public const string ScreenshotsDirectoryName = "Screenshots"; | ||
|
||
public static string GetTempSubDirectoryPath(string contextId, string subDirectoryName) => |
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.
I see you pass string.Empty
to this below. Would it be nicer to make ""
the default value of the subDirectoryName
argument? Or better yet, make it params string[] subDirectoryNames
?
public static string GetTempSubDirectoryPath(string contextId, string subDirectoryName) => | ||
Path.Combine(Environment.CurrentDirectory, Temp, contextId, subDirectoryName); | ||
public static string GetTempSubDirectoryPath(string contextId, params string[] subDirectoryNames) => | ||
Path.Combine(new[] { Environment.CurrentDirectory, Temp, contextId }.Union(subDirectoryNames).ToArray()); |
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.
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.
Right.
OSOE-98