rabbitmq / rabbitmq-dotnet-client

RabbitMQ .NET client for .NET Standard 2.0+ and .NET 4.6.2+
https://www.rabbitmq.com/dotnet.html
Other
2.07k stars 576 forks source link

Support netstandard2.0 (and higher) #686

Closed lukebakken closed 4 years ago

lukebakken commented 4 years ago

https://docs.microsoft.com/en-us/dotnet/standard/net-standard

@bording approved 😄

stebet commented 4 years ago

Does this mean you'll drop support for anything lower than netstandard2.0? If so, I'm all for it, so the codebase can move forward.

michaelklishin commented 4 years ago

Yes, that's exactly what it means @stebet.

stebet commented 4 years ago

That's great. In that case, I have a PR well on it's way that modernizes quite a bit, will probably submit it tomorrow :)

lukebakken commented 4 years ago

@stebet - great! Thanks.

bording commented 4 years ago

To make sure my comment on the other issue is clear, I'm currently thinking this should be the new TargetFrameworks value for the project:

<TargetFrameworks>net452;netstandard2.0</TargetFrameworks>
lukebakken commented 4 years ago

Some useful links to include in this project:

stebet commented 4 years ago

To make sure my comment on the other issue is clear, I'm currently thinking this should be the new TargetFrameworks value for the project:

<TargetFrameworks>net452;netstandard2.0</TargetFrameworks>

Why 4.5.2? If you are aiming for a major revision, I'd highly recommend moving the minimum target up to 4.6 at least, maybe even netstandard2.0 as a minimum. System.IO.Pipelines for example has 4.6 as the minimum requirement: https://www.nuget.org/packages/System.IO.Pipelines/

For added reference, the peeps at StackExchange will probably make netstandard2.1 a minimum for major revisions starting in 2020: https://twitter.com/marcgravell/status/1214226671550775297

NetFX support is dropping fast, and target anything lower than 4.6 will limit the possibilities of getting new and improved APIs pretty rapidly as NetFX will not be receiving any new features, since .NET 5.0 is being worked on.

lukebakken commented 4 years ago

@stebet - we would consider requiring a more recent .NET Framework if concrete benefits to users of this library can be demonstrated (faster performance, better memory use, etc).

stebet commented 4 years ago

As for benefits, a lot of them are well documented. I'd almost say that enabling the use of System.IO.Pipelines and System.Threading.Channels (both have 4.6 as minimum) alone would be reason enough. Also a lot of *Async functionality isn't available except in higher versions. More knowledgeable people like @benaadams, @davidfowl and @mgravell can probably point at several as well. I think 4.6.1 is a good middle ground between maintaining backwards compat (if that's a hard requirement) and enabling new/improved constructs and functionality.

https://docs.microsoft.com/en-us/dotnet/framework/whats-new/

lukebakken commented 4 years ago

OK thanks. I'm assuming that would require code changes. I am happy to run performance tests on proposed changes that are 4.6.1+ specific if you'd like to include them in your current PR or a new one. Thanks.

michaelklishin commented 4 years ago

I personally vote for going with 4.6.1 as the minimum supported version.

@bording would that work for your team?

On Thu, Jan 16, 2020 at 7:10 PM Luke Bakken notifications@github.com wrote:

OK thanks. I'm assuming that would require code changes. I am happy to run performance tests on proposed changes that are 4.6.1+ specific if you'd like to include them in your current PR or a new one. Thanks.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rabbitmq/rabbitmq-dotnet-client/issues/686?email_source=notifications&email_token=AAAAIQRNSWRWRC3SBYFU743Q6CBF5A5CNFSM4KHGJ6OKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJETRJA#issuecomment-575223972, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAIQSTKHDD45MYLK6VHF3Q6CBF5ANCNFSM4KHGJ6OA .

-- MK

Staff Software Engineer, Pivotal/RabbitMQ

stebet commented 4 years ago

OK thanks. I'm assuming that would require code changes. I am happy to run performance tests on proposed changes that are 4.6.1+ specific if you'd like to include them in your current PR or a new one. Thanks.

No real code changes necessary as is I think.

bording commented 4 years ago

@bording would that work for your team?

We currently ship with net452;netstandard2.0, though we are likely to reevaluate that when it's time for us to release a new major version of our core package.

For our rabbit transport, we'll have to release a new major version to support 6.0 anyway, so it would be okay to have to bump it up to 4.6.1 as part of that.

lukebakken commented 4 years ago

Addressed by #690