bjorkstromm / depends

Tool for generating dependency trees for .NET projects
MIT License
537 stars 41 forks source link

Support .NET 5 #23

Closed amaechler closed 2 years ago

amaechler commented 2 years ago

Closes #21.

This PR attempts to address #21 as part of Hacktoberfest 🎃.

I started by added net5.0 to the list of target frameworks as suggested by @bjorkstromm. After, I ended up having to do some initial steps:

There are still some warnings in the project, which I however believe to be out-of-scope for this PR. Specifically:

One option to address the above would be to only support .NET 5+ forward and release an updated version with said support. Older projects could then simply use a previous version of depends until updated.

bjorkstromm commented 2 years ago

I think we could remove support for netcoreapp2.1 also. Do you want to send a PR for that?

amaechler commented 2 years ago

That was quick, thanks @bjorkstromm.

I think we could remove support for netcoreapp2.1 also.

What do you think about removing support for both netcoreapp2.1 and netcoreapp3.1? As it only changes the target of the CLI application itself, it should still be able to analyze applications that were built using .NET Core 3.

To verify this, I quickly created a new a new project and analyzed its dependencies successfully:

 # in ./somewhere
 dotnet new console -n test -f netcoreapp3.1

 # in depends
 dotnet run --project .\src\Depends --framework net5.0 -- ./somewhere/test

Am I missing something, or does that make sense?

bjorkstromm commented 2 years ago

@amaechler the built framework does not decide what kind of project depends can analyze. It's using MSBuild behind the scenes. The built framework however must be installed on the machine for running the depends. netcoreapp3.1 is LTS so we'd like to support that (in case user doesn't have net5.0 installed). net5.0 is current, so we like to support that as well. In a couple of weeks when net6.0 is released (will be LTS), we'd like to support that as well.

amaechler commented 2 years ago

Ok, that makes sense. Thanks for clarifying.