Skip to content
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

fix incorrect as usage in ProjectFactory #6191

Conversation

SimonCropp
Copy link
Contributor

Bug

Fixes:

Description

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@SimonCropp SimonCropp requested a review from a team as a code owner December 14, 2024 12:04
@microsoft-github-policy-service microsoft-github-policy-service bot added Community PRs created by someone not in the NuGet team labels Dec 14, 2024
nkolev92
nkolev92 previously approved these changes Dec 18, 2024
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

Can we please get an issue created for this and the PR template filled out?

Nigusu-Allehu
Nigusu-Allehu previously approved these changes Dec 18, 2024
@Nigusu-Allehu Nigusu-Allehu dismissed stale reviews from nkolev92 and themself via c7db036 December 18, 2024 18:11
@Nigusu-Allehu Nigusu-Allehu enabled auto-merge (squash) December 18, 2024 19:59
Nigusu-Allehu
Nigusu-Allehu previously approved these changes Dec 18, 2024
@@ -143,24 +143,15 @@ private void Initialize(dynamic project)
}

// This happens before we obtain warning properties, so this Logger is still IConsole.
IConsole console = Logger as IConsole;
switch (LogLevel)
if (Logger is IConsole console)
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of tests were failing because Logger was NullLogger which resulted in an invalid cast exception. Instead of as we could use is and do a onetime check if the Logger is an IConsole type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another thing we could do is

if (LogLevel != LogLevel.Debug)
{
    var console = (IConsole)Logger;
    console.Verbosity = LogLevel switch
    {
        LogLevel.Verbose => Verbosity.Detailed,
        LogLevel.Information => Verbosity.Normal,
        LogLevel.Minimal => Verbosity.Quiet,
        _ => console.Verbosity
    };
}

And in our tests make sure LogLevel == LogLevel.Debug

@SimonCropp SimonCropp closed this Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PRs created by someone not in the NuGet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants