Skip to content

Add new properties for analyzer#16098

Merged
tlakollo merged 7 commits intodotnet:masterfrom
tlakollo:NewAnalyzerProperties
Mar 10, 2021
Merged

Add new properties for analyzer#16098
tlakollo merged 7 commits intodotnet:masterfrom
tlakollo:NewAnalyzerProperties

Conversation

@tlakollo
Copy link
Contributor

Add a layer of indirection for enabling the analyzer. By using AnalyzeSingleFile and AnalyzeTrimming we open up the usage of the analyzer for other platforms. For example, Xamarin doesn't use the flag PublishSingleFile but could set AnalyzeSingleFile to run the analyzer.

@tlakollo tlakollo requested review from agocke and sbomer February 26, 2021 03:11
@tlakollo tlakollo self-assigned this Feb 26, 2021
@tlakollo tlakollo marked this pull request as draft February 26, 2021 09:02
@tlakollo tlakollo marked this pull request as ready for review February 26, 2021 22:17
Reorg properties so they can use info not set in these targets
@sbomer
Copy link
Member

sbomer commented Mar 2, 2021

Thinking about the naming - I wonder if they should be called EnableSingleFileAnalysis and EnableTrimAnalysis to match the existing EnableNetAnalyzers.

@samsp-msft do you have any feedback on the new property names? (context: they are used to enable the Roslyn single-file/trim analysis even if not using PublishSingleFile/PublishTrimmed.)

@tlakollo
Copy link
Contributor Author

tlakollo commented Mar 4, 2021

I was thinking that might be good to stick to naming like EnableNetAnalyzers and call them EnableSingleFileAnalyzers and EnableTrimAnalyzers
Edit: Using the singular might also be an option -> EnableSingleFileAnalyzer

@sbomer
Copy link
Member

sbomer commented Mar 4, 2021

I like the singular EnableSingleFileAnalyzer and EnableTrimAnalyzer

Copy link
Member

@sbomer sbomer left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. Thanks!

@tlakollo tlakollo merged commit ac7fc4f into dotnet:master Mar 10, 2021
@tlakollo tlakollo deleted the NewAnalyzerProperties branch March 10, 2021 05:53
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.

2 participants