Sounds good! Yeah, this is a bit of an interesting case where code will still compile against the new API without change, but I think maintaining compatibility of the binary API is also important. Thanks for looking into it.
-Dan On Tue, May 14, 2019 at 11:22 AM Ryan McMahon <rmcma...@pivotal.io> wrote: > Hi all, > > This is my mistake - it was a misunderstanding of what constitutes a > breaking public API change. If a client app were to recompile using the > new bits with the new method signature, there wouldn't be an issue because > the new signature would work with 0 varargs. The problem arises when you > hot swap the geode dependencies for that client app without recompiling, as > the bytecode no longer matches. Since we do support the hot swap use case > for CVEs etc, I see now this is considered a breaking API change. > > I'll change this to use a method overload instead, it should be a pretty > simple fix. Luckily, this issue didn't make it into any Geode releases. > > Thanks, > Ryan > > > On Tue, May 14, 2019 at 10:47 AM Udo Kohlmeyer <u...@apache.org> wrote: > > > How did this slip the review process that the signature of a public API > > was changed? > > > > Well done Gester for finding this!! > > > > --Udo > > > > On 5/14/19 10:40, Dan Smith wrote: > > > I think the changes for GEODE-6327 broke backwards compatibility in > > > JSONFormatter with the change from fromJSON(String jsonString) to > > > fromJSON(String jsonString, String... identityFields) > > > > > > Adding an additional varargs parameter to the method breaks code that > was > > > compiled against the non-varargs version. I think we need to overload > the > > > method instead. > > > > > > Thanks to Gester for discovering this with his test! > > > > > > -Dan > > > > > >
