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 reporting of MSBuild warnings in MSBuildWorkspace #75183

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

svick
Copy link
Contributor

@svick svick commented Sep 20, 2024

Fixes #75182

@svick svick requested a review from a team as a code owner September 20, 2024 10:07
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 20, 2024
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Sep 20, 2024
@svick svick added IDE-MSBuildWorkspace MSBuildWorkspace and removed VSCode labels Sep 20, 2024
}
}

private static string GetMSBuildFailedMessage(string projectFilePath, string message)
private static string GetMSBuildFailedMessage(string projectFilePath, string message, WorkspaceDiagnosticKind diagnosticKind)
=> RoslynString.IsNullOrWhiteSpace(message)
Copy link
Member

Choose a reason for hiding this comment

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

It might just be easier to fill in a blank message with "no message" or something so the log entry would be a bit clearer we're missing one (and it reduces the number of strings we have). That said, happy to keep this as is since you're just maintaining behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the change.

<value>MSBuild failed when processing the file '{0}' with message: {1}</value>
</data>
<data name="Msbuild_warning_when_processing_the_file_0" xml:space="preserve">
<value>MSBuild warning when processing the file '{0}'</value>
Copy link
Member

@JoeRobich JoeRobich Sep 20, 2024

Choose a reason for hiding this comment

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

I think we should change this string to MSBuild warned ... to keep it in line with the MSBuild failed ... string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@tmat
Copy link
Member

tmat commented Sep 21, 2024

Will this change fix #75170?

@svick
Copy link
Contributor Author

svick commented Oct 14, 2024

@tmat I'm not sure, but probably not.

If the issues are originally reported as warnings, then this PR will make sure they will end up as warnings. Otherwise, it won't change anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. IDE-MSBuildWorkspace MSBuildWorkspace untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSBuildWorkspace reports warnings as errors/failures
5 participants