eventflow / EventFlow

Async/await first CQRS+ES and DDD framework for .NET
https://docs.geteventflow.net/
Other
2.33k stars 442 forks source link

Migrate Event Store & System.Text.Json #985

Closed kyle-bradley closed 9 months ago

kyle-bradley commented 1 year ago

I by mistake combined these two changes into one, but they are related so haven't taken the effort in separating them - unless it is requested.

See https://github.com/eventflow/EventFlow/pull/983 For the description on System.Text.Json changes.

I upgraded Event Store and took the opportunity to upgrade most of the other packages. MsSQL and PostgreSQL being the exception as those would be a larger migration.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

rasmus commented 1 year ago

I'll have a look On 8 May 2023, at 16.28, CLAassistant @.***> wrote: Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.Kyle Bradley seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.You have signed the CLA already but the status is still pending? Let us recheck it.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

kyle-bradley commented 1 year ago

Sorry about that, had the wrong email address linked

github-actions[bot] commented 9 months ago

Hello there!

We hope this message finds you well. We wanted to let you know that we have noticed that there has been no activity on this pull request for the past 90 days, which makes it a stale pull request.

As a result, we will be closing this pull request within the next seven days. If you still think this pull request is necessary or relevant, please feel free to update it or leave a comment within the next seven days.

Thank you for your contributions and understanding.

Best regards, EventFlow

github-actions[bot] commented 9 months ago

Hello there! I'm a bot and I wanted to let you know that your pull request has been closed due to inactivity after being marked as stale for seven days. If you believe this was done in error, or if you still plan to work on this pull request, please don't hesitate to reopen it and let us know. We're always happy to review and merge high-quality contributions. Thank you for your interest in our project! Best regards, EventFlow

Jaben commented 3 months ago

@rasmus So, no upgrade to System.Text.Json?

rasmus commented 3 months ago

@Jaben it's the one thing that I'm most scare of doing, simply removing the attributes from the values object classes. Doing this can potentially break it for a lot of people and with everything going on with the 1.0 preparation I'm looking to push this to a 2.0 release instead.

However, I will be looking at this PR and another attempt that has been done in order to add System.Text.Json support baked into 1.0 at "launch". The only downside of this, is that any releases that use System.Text.Json will have a dead Newtonsoft.Json DLL along for the ride. Yes, not optimal, but people that really want System.Text.Json get it and people on Newtonsoft.Json gets an easier upgrade path.

Again, this is a change I have had a really, really hard figuring out how to make every one happy and merely having the dead DLL seems like the lesser evil for the 1.0 release.

Jaben commented 3 months ago

@rasmus Thanks for the response! It does look like the PR(s) have massive surface areas. I understand the concern for backward compatibility and that it's not necessarily worth tackling for 1.0 as it's a feature. But I wouldn't mind the dead "Newtonsoft.Json" dependency option as a half-step.