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

Make Ignore Warnings suppress the output level entirely #4221

Merged
merged 15 commits into from
Mar 15, 2024

Conversation

Trenly
Copy link
Contributor

@Trenly Trenly commented Mar 1, 2024

Instead of handling each case individually, this PR makes the --ignore-warnings a common argument and suppresses all output from context.Reporter.Warn(). Support is added for enabling or disabling each reporter level individually.

cc @yao-msft

Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly requested a review from a team as a code owner March 1, 2024 02:24
Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

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

Thanks to previous build test failures, we can now truly "discard" the previous pr :)

src/AppInstallerCLICore/ExecutionReporter.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/ExecutionReporter.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/ExecutionReporter.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/ExecutionReporter.cpp Outdated Show resolved Hide resolved
@denelon
Copy link
Contributor

denelon commented Mar 1, 2024

Thanks @kaleb with a FANTASIC improvement for users! ❤️

mdanish-kh

This comment was marked as resolved.

@riotrah
Copy link

riotrah commented Mar 2, 2024

Sorry for adding noise - I just wanted to say I'm really impressed with and appreciate the insane response time to #4213

src/AppInstallerCLICore/ExecutionReporter.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/PromptFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/ExecutionReporter.h Outdated Show resolved Hide resolved
@yao-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

yao-msft
yao-msft previously approved these changes Mar 14, 2024
@Trenly
Copy link
Contributor Author

Trenly commented Mar 14, 2024

Thanks for all your patience and guidance @yao-msft, It really helps me understand a lot more about why certain things have been done the way they are

@yao-msft
Copy link
Contributor

@Trenly I cannot merge due to merge conflicts. Can you merge to latest when you have time? Thanks.

@Trenly
Copy link
Contributor Author

Trenly commented Mar 14, 2024

@Trenly I cannot merge due to merge conflicts. Can you merge to latest when you have time? Thanks.

Should be good now

@yao-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft yao-msft merged commit cf597ec into microsoft:master Mar 15, 2024
8 checks passed
@Trenly Trenly deleted the SuppressOutputLevels branch March 15, 2024 04:22
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.

Export should allow suppressing missing source warnings
6 participants