Skip to content

Conversation

@JoeRobich
Copy link
Member

@JoeRobich JoeRobich commented Oct 3, 2019

These changes setup the Correctness Test leg to fail when the formatting analyzer reports warnings. Other test legs will ignore these warnings and run unit tests as expected. Local builds (.\build.cmd and VS) will report warnings but not fail the build.

@JoeRobich JoeRobich added Area-Infrastructure PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. labels Oct 3, 2019
@JoeRobich
Copy link
Member Author

JoeRobich commented Oct 3, 2019

Build logs showing a failing correctness build due to formatting errors. Other legs successfully built and started running tests.

src\Workspaces\Core\Portable\AddImports\AbstractAddImportsService.cs(84,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting
src\Workspaces\Core\Portable\AddImports\AbstractAddImportsService.cs(145,121): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting
src\Compilers\CSharp\Portable\BoundTree\UnboundLambda.cs(620,13): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting
src\Compilers\CSharp\Portable\BoundTree\UnboundLambda.cs(868,17): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting
Build failed.
Cmd.exe exited with code '1'.

@JoeRobich JoeRobich removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Oct 3, 2019
@JoeRobich JoeRobich marked this pull request as ready for review October 3, 2019 18:11
@JoeRobich JoeRobich requested review from a team as code owners October 3, 2019 18:11
@JoeRobich
Copy link
Member Author

@sharwell & @jaredpar please take a look

@jaredpar
Copy link
Member

jaredpar commented Oct 4, 2019

Overall looks good. Think we need to also make the following changes:

When building the desktop legs we should not pass warnAsError and should pass skipAnalyzers. That way the desktop leg is focused on running teests while the correctness leg is focused on build correctness including analyzers, formatting, etc ....

https://github.com/dotnet/roslyn/blob/master/azure-pipelines.yml#L40

We should be able to enable MSBuild's warn as error without qualification now here

https://github.com/dotnet/roslyn/blob/master/eng/build.ps1#L231

@JoeRobich
Copy link
Member Author

JoeRobich commented Oct 8, 2019

Build logs showing failing correctness leg

src\Features\CSharp\Portable\SignatureHelp\InitializerExpressionSignatureHelpProvider.cs(25,185): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting

@JoeRobich
Copy link
Member Author

@jaredpar Did I get those changes right?

Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

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

:shipit:

@RikkiGibson RikkiGibson merged commit 2ae01f8 into dotnet:master Oct 9, 2019
@RikkiGibson
Copy link
Member

I have been instructed by the author to hit the button

@JoeRobich JoeRobich deleted the enable-warnaserror branch February 7, 2020 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants