Skip to content

Re-enable CoreLib analyzers - #424

Merged
ViktorHofer merged 3 commits into
dotnet:masterfrom
ViktorHofer:EnableCoreLibAnalyzers
Dec 2, 2019
Merged

Re-enable CoreLib analyzers#424
ViktorHofer merged 3 commits into
dotnet:masterfrom
ViktorHofer:EnableCoreLibAnalyzers

Conversation

@ViktorHofer

@ViktorHoferViktorHofer commented Dec 1, 2019

Copy link
Copy Markdown
Member

CoreLib analyzers were unintentionally disabled with
6afe96c. This re-enables the analyzers
and conditions one which brings in external dependencies that CoreLib
can't deal with.

Fixes#422

@ViktorHofer
Re-enable CoreLib analyzers
5e58dcf
CoreLib analyzers were unintentionally disabled with
6afe96c. This re-enables the analyzers
and conditions one which brings in external dependencies that CoreLib
can't deal with.
@ViktorHoferViktorHofer added the area-Infrastructure-coreclr Only use for closed issues label Dec 1, 2019
@ViktorHoferViktorHofer self-assigned this Dec 1, 2019
stephentoub
stephentoub reviewed Dec 1, 2019
View reviewed changes
Comment threadeng/Analyzers.props Outdated
@ViktorHoferViktorHofer mentioned this pull request Dec 1, 2019
Merged
jkotas
jkotas reviewed Dec 1, 2019
View reviewed changes
Comment threadeng/Analyzers.props Outdated
@ViktorHofer

ViktorHofer commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

I'll remove that package as its description clearly state that it shouldn't be referenced directly. @safern do you have context why that package was referenced directly in corefx/arcade?

@ViktorHofer
Remove Microsoft.CodeAnalysis.Common from common
7ec592f
Microsoft.CodeAnalysis.Common shouldn't be referenced directly,
therefore removing that package from the common location and only
restoring and marking it as an analyzer in the libraries restore phase.
@ViktorHofer

ViktorHofer commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

OK, so the transitive dependency to Microsoft.CodeAnalysis.Common is required becaues of the special way we add analyzers to projects in libraries. We will clean this up with @Anipik's work to enable project restore for non test projects in libraries.

jkotas
jkotas reviewed Dec 2, 2019
View reviewed changes
Comment threadsrc/libraries/restore/analyzers/analyzers.depproj Outdated
jkotas
jkotas reviewed Dec 2, 2019
View reviewed changes
Comment threadeng/Analyzers.props
@ViktorHofer

ViktorHofer commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

This should be ready, can someone pls approve? Thanks

@ViktorHofer

ViktorHofer commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

Thanks, merging as the previous run was green (except an unrelated test failure) and I just changed a comment which re-triggered CI.

@ViktorHofer
ViktorHofer merged commit f84d01b into dotnet:masterDec 2, 2019
@ViktorHofer
ViktorHofer deleted the EnableCoreLibAnalyzers branch December 2, 2019 17:13
safern
safern reviewed Dec 2, 2019
View reviewed changes

@safernsafern left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

safern pushed a commit that referenced this pull request Dec 3, 2019
@ViktorHofer@safern
Re-enable CoreLib analyzers (#424)
83232d6
* Re-enable CoreLib analyzers
CoreLib analyzers were unintentionally disabled with
6afe96c. This re-enables the analyzers.
* Remove Microsoft.CodeAnalysis.Common from common
Microsoft.CodeAnalysis.Common shouldn't be referenced directly,
therefore removing that package from the common location and only
restoring and marking it as an analyzer in the libraries restore phase.
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotasjkotasjkotas left review comments
@safernsafernsafern left review comments
@stephentoubstephentoubstephentoub approved these changes

Assignees

@ViktorHoferViktorHofer

Labels

area-Infrastructure-coreclrOnly use for closed issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Re-enable analyzers for System.Private.CoreLib

4 participants

@ViktorHofer@stephentoub@jkotas@safern