Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Add new roslyn code analyzers to support optimal Span/Memory uses within upstream repos. #2179

Closed
ahsonkhan opened this issue Oct 4, 2018 · 5 comments

Comments

@ahsonkhan
Copy link
Member

ahsonkhan commented Oct 4, 2018

We have prototype analyzers within corefxlab atm which we can use as a starting point that encapsulates the tips from here: https://www.codemag.com/Article/1807051/Introducing-.NET-Core-2.1-Flagship-Types-Span-T-and-Memory-T

Once we add them to buildtools, upstream repos (like corefx/coreclr/etc.) will automatically throw warnings during build (which are generally treated as errors) and could even get suggested code fixes going forward.

dotnet/corefxlab#2206
dotnet/corefxlab#2236

ML.NET has their own internal code analyzers and we should consider how to support that (and potentially other) repos as well:
https://github.com/dotnet/machinelearning/tree/master/tools-local/Microsoft.ML.InternalCodeAnalyzer

cc @KrzysztofCwalina, @joshfree, @weshaggard, @eerhardt, @terrajobst

@ahsonkhan ahsonkhan self-assigned this Oct 4, 2018
@ahsonkhan ahsonkhan added this to the Future milestone Oct 4, 2018
@eerhardt
Copy link
Member

eerhardt commented Oct 4, 2018

buildtools is going away. I think this issue should be opened in https://github.com/dotnet/arcade.

@ahsonkhan
Copy link
Member Author

ahsonkhan commented Oct 4, 2018

I talked to @weshaggard about this and he suggested keeping it in buildtools for the time being with the rest of the analyzers until we have a plan to move them all together.

@eerhardt
Copy link
Member

eerhardt commented Oct 4, 2018

It would seem that folks like the SDK team and ASP.NET would benefit from these analzyers as well. Getting them in arcade sooner would speed that up IMO.

Why put something here now, when you know it will be moved in less than a month or two?

@weshaggard
Copy link
Member

@eerhardt it is just adding additional analyzers to our existing set so whether or not that it is done here or in arcade I don't think it makes much of a difference.

That said if @ahsonkhan is willing to do the work-item to move the analyzers to arcade as part of this work I'm all for it :)

@ahsonkhan ahsonkhan removed their assignment Sep 11, 2019
@ViktorHofer
Copy link
Member

// Auto-generated message

Going forward, the .NET team is using https://github.com/dotnet/arcade to develop the code and issues formerly in this repository. Feel free to reopen/move this issue if it still applies to servicing branches in this repository, or source code which now resides in the Arcade repository.

@ViktorHofer ViktorHofer removed this from the Future milestone Mar 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants