search

MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
56 stars 14 forks source link

BuildQualityChecks Warning Selectors does not work #238

Closed szabolcsleitner closed 1 month ago

szabolcsleitner commented 2 months ago

Describe the context

Describe the problem and expected behavior We have tried to use the Warning Selectors (Tasks), but it does not work. We want to keep every warning except one specific id: /^##\[warning\]((?!MSB3491).)*$/i but all warning are ignored every time. Did we miss something? We followed the docs: https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/WarningsPolicy.md#warningSelectors

ReneSchumacher commented 2 months ago

Hi @szabolcsleitner,

warning selectors cannot be used to ignore warnings. Instead, you tell BQC to only evaluate warnings matching your selector expressions. So, while it is theoretically possible to create a set of regular expressions that only match the relevant warnings, it is not recommended to it that way. Instead, you should use language and compiler constructs to disable warnings you want to ignore (e.g., suppress code analysis warnings, disable warnings using a #pragma preprocessor directive, or ignore warnings using compiler options).

szabolcsleitner commented 2 months ago

But we want to see these warnings, just do not want to count in Build Quality.

ReneSchumacher commented 1 month ago

Hi again,

unfortunately, we haven't implemented any filters for warnings in our task. We have discussed about this years ago and decided against it. We may add them in the future, but I'm wondering about your use case. Why would you want to have warnings that are not counted by BQC?

Our goal was to help people set some kind of maximum amount of technical debt or help them reduce their warnings steadily to reduce technical debt. Once warnings are at zero and should stay at zero, our recommendation has always been to treat warnings as errors in the compiler/transpiler settings and not rely on BQC unless there is no option to treat warnings as errors.

In your case, it seems that you want to "ignore" some warnings but still want to see them. One way to achieve this with BQC would be setting a warning threshold that accounts for the "ignored" warnings. But if that type of warnings may increase over time, I don't see a reason for showing them anyway. Then, I believe it would be better to really ignore them.

Maybe you can help me understand your use case better?

szabolcsleitner commented 1 month ago

In general, I fully agree with you.

I'll try to explain our use case. We have finally reduced the warning count in our repo to zero. And we want to keep it (not just like the three previous attempt where we used to believed that the devs can maintain it themself. They can't). After then we have looked for a solution how to check and force it. We have set up the treat warnings as errors (exclude some well known or internal dev time warnings) and found this awesome task. Worked well, untill the randomness hits. Yes, a random warning. This MSB3491 comes from a SQLDB sln randomly and messes up the CI builds. (by the way the MSB3491 originally is an error, but in the official SDK it ranked down as warning. Crazy (?)). In the current scenario we have decided to ignore this warning in this specific case in this specific solution. But I am not a fan this kind of suppress. A warning is a warning, and most of the cases it can explain a strange outcome. Even this one. A suppress can be totally okay. Every codebase contain warning suppressions (eg. a pragma with a explanation comment). But a full generic suppress... not on my watch. Again, I agree with you. But in this scenario, until we found out how can we stabilize a random warning, we would like to not mess up the build, but we need to see the warnings. Therefore a warning threshold is not a solution, because it is also generic, it will allow other warnings.

This warning is clearly not critical. But I want to stabilize it long term. But, I am afraid that it is just the tip of the ice(warning)berg. And I would be calm, if we have a solution that is not a full (solution level) suppress.

ReneSchumacher commented 1 month ago

Hm, got it. Looks like a pretty special case to me. Let's see if I can quickly add warning exclusions to the task.

ReneSchumacher commented 1 month ago

Hi @szabolcsleitner,

we're in the process of releasing an update to BQC, which supports warning exclusions. Should be available in the marketplace within the next couple hours. Hope it meets your needs.

szabolcsleitner commented 1 month ago

Hi @ReneSchumacher ,

awesome, thank you :) I've configured it, and we'll see after a few runs.

Thanks again!

szabolcsleitner commented 1 month ago

It works as expected. Thank you again @ReneSchumacher !

ReneSchumacher commented 1 month ago

@szabolcsleitner Great, thanks for confirming and thank you for using our task. Please let us know if you need anything else.