-
Notifications
You must be signed in to change notification settings - Fork 841
Fix warning breaking official build, enable warningAsError in all pipelines #6988
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
Conversation
And make source-index step use similar build entrypoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes build configuration improvements and adds a workaround for .NET Framework builds:
- Removes the
warnAsError: 0override from CI pipelines to enforce warnings as errors consistently - Adds a custom source indexing build command for main branch builds
- Includes a workaround for Microsoft.Bcl.AsyncInterfaces dependency issue on .NET Framework
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/Libraries/Microsoft.Extensions.DataIngestion.Tests/Microsoft.Extensions.DataIngestion.Tests.csproj | Adds conditional package reference for Microsoft.Bcl.AsyncInterfaces to work around Semantic Kernel issue #13316 on .NET Framework |
| azure-pipelines.yml | Removes warnAsError override and adds sourceIndexBuildCommand for source indexing on main branch |
| azure-pipelines-unofficial.yml | Removes warnAsError override to enforce warnings as errors |
| azure-pipelines-public.yml | Removes warnAsError override to enforce warnings as errors |
|
@joperezr let me know that this is supposed to be handled by
|
This allows folks to get test results without being blocked by warnings. Make sure the job testing for warnings runs on windows to ensure it builds a superset of targets.
adamsitnik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big thanks for solving the problem and providing all the explanation @ericstj !
And apologies for introducing the problem.

Fixes #6986
I discovered that all builds had disabled warning as error except the source-index step, which went through the arcade script instead of the repo script.
A warning was introduced in 5ca1721 that caused official build to fail due to source-index job.
I've left warning as error disabled for the build scripts (local developer scenario) but I've made all the AzDo pipelines enable warnAsError.
These warnings are important, the one here would mean actual runtime problems if it were in a product assembly. microsoft/semantic-kernel#13316 I'm not sure why the test didn't hit it, maybe it happens to not call any API that use async interfaces on netfx.
Microsoft Reviewers: Open in CodeFlow