-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Revert #5416, broke DI backward compatibility #5454
Conversation
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
I liked #5416 on balance, but due to the issues I identified in #5453 we can't let this hang out there in the open. Broke every Akka.DI plugin. Even though we're deprecating those we haven't communicated that to end users or set an explicit deadline. Can't break things by accident in a revision release like that. |
What exactly break and why do we revert racy spec and bug fixes too? If its only the IActorProducer interface we could change it back and use a better binary compatible version/pattern |
We're reverting the entire PR because it was the most expedient way to fix the error. We can't tolerate a surprise breaking change like that affecting users' applications with no warning. I would love to reintroduce your changes but they need to be done in smaller chunks; going forward I'm going to be more aggressive about rejecting PRs that get too big and touch too many things. Shouldn't be racy spec fixes and other bug fixes on that same PR that touches core API changes if those changes are unrelated. Could you please reintroduce those in smaller PRs? As I mentioned here #5453 a few moments ago, I'm going to document our standards explicitly going forward. We have to. |
For the record, this breaking change and the subsequent revert is my fault - I haven't helped us build an API change management system that is more resilient, I haven't documented the standards we need to make it easy to contribute to the project, I approved the PR. I'm going to bump that up to the top of my list for Jan 2022. |
I'm sorry for making the process of contributing to Akka.NET more frustrating that necessary for you @Zetanova - that's my fault too. I'll put a dent in https://github.com/akkadotnet/akka.net/projects/7 and update |
We expected that it will not binary compatible with Akka.DI, you wanted to test it and then it was merged. I will copy the fixes out, but not this week. What we need is also a good system to mark things obsolete and to know when they can be deleted. |
You're absolutely right and that's on me. |
Agreed - we usually do this between minor versions, but again - I've not documented that. |
Ironically I -do- still use Akka.DI, although this hasn't yet impacted us yet as we are a little more conservative on non-critical package upgrades. |
No description provided.