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

pass location information to EventArgs #10545

Merged

Conversation

JanProvaznik
Copy link
Contributor

@JanProvaznik JanProvaznik commented Aug 21, 2024

Fixes #10529

Context

Changes Made

split IMSBuildElementLocation to separate file and added it to Framework .dll (kept in Microsoft.Build.BackEnd namespace, that should be moved eventually if desirable #10544 )
removed location from BuildCheckResult.FormatMessage, location is logged via the filled parameters by Build*EventArgs

Testing

Notes

add Build*EventArgs constructors for relevant data
split IMSBuildElementLocation to file
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Looks good!

Can you add sample output of the buildcheck console outputs?

@JanProvaznik
Copy link
Contributor Author

Before

image

After

image

@JanProvaznik JanProvaznik marked this pull request as ready for review August 23, 2024 08:57
@JanKrivanek
Copy link
Member

We should check the appearance of the suggestion level as well, but otherwise this feels ready to go!

Copy link
Contributor

@maridematte maridematte left a comment

Choose a reason for hiding this comment

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

It would be nice to have some tests to check if the location is being passed right from the check to the output.

src/Framework/BuildWarningEventArgs.cs Show resolved Hide resolved
@YuliiaKovalova
Copy link
Member

It would be nice to have some tests to check if the location is being passed right from the check to the output.

I moved it to a separate ticket: #10581

@YuliiaKovalova YuliiaKovalova merged commit 01e53f4 into dotnet:main Aug 29, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BuildCheck UX: properly pass location information to EventArgs
5 participants