-
Notifications
You must be signed in to change notification settings - Fork 990
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
Make FormParserFactory builder more fluent #726
Conversation
Can one of the admins verify this patch? |
Changing the return type of a method is transparent at compile time, but I believe it is a runtime breaking change, even if the caller does nothing with the return value. |
Ah, I didn't consider binary compatibility. Perhaps we can consider this for the next major upgrade? |
Instead of modifying the existing methods, I've committed a new change that adds two new methods with the prefix |
Adding a return type to a previously void methods would only be an ABI break for types which extend the builder, which seems very unlikely. Usages of the builder will be unaffected. |
This is ok to test |
The return type is used in the bytecode to specify which method a binary is invoking so yes, we could break binaries that were compiled with previous version if we change the return type in the signature. I'm okay with the "with" new methods. |
I created a Jira for this: https://issues.jboss.org/browse/UNDERTOW-1520 |
Windows Build 2726 outcome was FAILURE using a merge of 2cee334 Failed tests
|
Windows Build 2729 outcome was FAILURE using a merge of cb887f5 Failed tests
|
Linux Build 2978 outcome was FAILURE using a merge of cb887f5 |
@unswirl thanks for adding the commit. Still, I need a single commit. For now, I'll try to squash your commits and add them to undertow myself, just to try to add them to the next release, which I'm about to do at any moment. |
A non-breaking change that makes the FormParserFactory.Builder methods chainable.