-
Notifications
You must be signed in to change notification settings - Fork 128
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
Port single-file analyzer #1665
Conversation
test/ILLink.RoslynAnalyzer.Tests/AvoidAssemblyLocationInSingleFileTests.cs
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/AvoidAssemblyLocationInSingleFileTests.cs
Outdated
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/AvoidAssemblyLocationInSingleFileTests.cs
Outdated
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/AvoidAssemblyLocationInSingleFileTests.cs
Outdated
Show resolved
Hide resolved
{ | ||
public static class MSBuildPropertyOptionNames | ||
{ | ||
public const string PublishSingleFile = nameof (PublishSingleFile); |
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.
We probably need to add PublishTrimmed
now since otherwise all the trim warnings will automatically be turned on during single-file
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.
Wouldn't be better to think about the more general area than to hardcode the single narrow scenario? Are we going to add a property for each of remaining scenario which would benefit from the same logic?
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.
Perhaps there's some broader feature work we could do, but we certainly need to recognize whether trimming or single-file are enabled, so we at least need to recognize the two MSBuild properties that already exist.
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.
LGTM, aside from missing test
test/ILLink.RoslynAnalyzer.Tests/AvoidAssemblyLocationInSingleFileTests.cs
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs
Outdated
Show resolved
Hide resolved
* Add single-file analyzer * Add tests * Use str constant * Update Resources * Update tests * PR feedback * Remove mysterious filename from comments * Add helper method for single-file diagnostic verification * Refactor MSBuild properties * Add test with PublishSingleFile not set Commit migrated from dotnet/linker@a86658f
No description provided.