-
Notifications
You must be signed in to change notification settings - Fork 325
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
Highest version filtering for extensions. #1051
Conversation
…found. Keep only the highest versioned extensions.
{ | ||
Dictionary<string, Tuple<Version, string>> extensionMap = new Dictionary<string, Tuple<Version, string>>(); | ||
|
||
foreach (var extensionFullPath in extensions) |
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.
foreach (var extensionFullPath in extensions) [](start = 12, length = 45)
Please try group by, and then get the highest file version within the group if the group size > 1
@@ -70,6 +73,13 @@ public FileAttributes GetFileAttributes(string path) | |||
} | |||
|
|||
/// <inheritdoc/> | |||
public Version GetFileVersion(string path) | |||
{ | |||
var currentFileVersion = FileVersionInfo.GetVersionInfo(path)?.FileVersion; |
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.
nit: null check not required
public Version GetFileVersion(string path) | ||
{ | ||
var currentFileVersion = FileVersionInfo.GetVersionInfo(path)?.FileVersion; | ||
return Version.TryParse(currentFileVersion, out var currentVersion) ? currentVersion : DefaultFileVersion; |
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.
May not work in UAP. We should move to PlatformAbstraction otherwise.
/// <returns>Filtered list of extensions</returns> | ||
private IEnumerable<string> FilterExtensionsBasedOnVersion(IEnumerable<string> extensions) | ||
{ | ||
var filteredExtensions = new List<string>(); |
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.
Add logging
|
||
var extensionGroups = extensions.GroupBy(ext => Path.GetFileNameWithoutExtension(ext)); | ||
|
||
foreach (var extensionGroup in extensionGroups) |
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.
optimize
[TestMethod] | ||
public void GetTestPlatformExtensionsShouldReturnPathToSingleFileExtensionOfATypeIfVersionsAreSame() | ||
{ | ||
List<string> sourcesDir = new List<string> { "C:\\Source1", "C:\\Source2" }; |
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.
DRY
{ | ||
if (extensionGroup.Count() > 1) | ||
{ | ||
var maxFile = extensionGroup.OrderByDescending(a => this.fileHelper.GetFileVersion(a)).First(); |
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.
Log message here.
private Version GetAndLogFileVersion(string path) | ||
{ | ||
var fileVersion = this.fileHelper.GetFileVersion(path); | ||
EqtTrace.Verbose("FileVersion for {0} : {1}", path, fileVersion); |
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.
Check if logging enabled.
Changes for filtering in case multiple extensions with same name are found. Keep only the highest versioned extensions.
Pending: Unit tests.