Skip to content

Conversation

tamasvajk
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the C# label Apr 24, 2024
@tamasvajk tamasvajk marked this pull request as ready for review April 24, 2024 11:10
@tamasvajk tamasvajk requested a review from a team as a code owner April 24, 2024 11:10
@@ -143,7 +143,7 @@ private static BuildScript DownloadDotNet(IBuildActions actions, ILogger logger,
// See https://docs.microsoft.com/en-us/dotnet/core/tools/global-json
var versions = new List<string>();

foreach (var path in files.Where(p => p.EndsWith("global.json", StringComparison.Ordinal)))
foreach (var path in files.Where(p => p.EndsWith(Path.DirectorySeparatorChar + "global.json", StringComparison.Ordinal)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you run into a case where a file was name myglobal.json or something similar?
nit: Maybe consider using Path.GetFileName? (same comment for the other similar change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggesting Path.GetFileName, that handles two other cases too. I'll change it...

I was putting together an integration test, and temporarily renamed the packages.config to x_packages.config, and then I was surprised that the packages were still picked up by the dependency manager.

michaelnebel
michaelnebel previously approved these changes Apr 24, 2024
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +206 to +210
catch (Exception ex)
{
logger.LogDebug($"Failed to get directory name for {path}: {ex.Message}");
return "";
}

Check notice

Code scanning / CodeQL

Generic catch clause

Generic catch clause.
Comment on lines +219 to +223
catch (Exception ex)
{
logger.LogDebug($"Failed to get file name for {path}: {ex.Message}");
return null;
}

Check notice

Code scanning / CodeQL

Generic catch clause

Generic catch clause.
@tamasvajk tamasvajk merged commit f29d2c2 into github:main Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants