+1 to not removing deprecated apis in minor releases. The semver policy Alexander describes seems reasonable. In the past of we have had something deprecated for a long time we have felt free to remove it whenever we want but I think the semver policy is a better way to decide when we are free to remove deprecated external apis.
On Wed, Nov 29, 2017 at 8:50 AM, Alexander Murmann <amurm...@pivotal.io> wrote: > Even though the class is deprecated, you should be able to go from one > minor version to another without having to worry about anything breaking. > The point of semver is to provide information about things breaking or not > without having to read the changelog. If we remove APIs in a minor version > because they were previously deprecated we break that contract. Semver.org > <https://semver.org/#how-should-i-handle-deprecating-functionality> > recommends > that the functionality should be marked as deprecated in a minor release > and then removed as part of a major release. > > On Tue, Nov 28, 2017 at 7:03 PM, Jacob Barrett <jbarr...@pivotal.io> > wrote: > > > Since the class was deprecated and is technically only there for pre-1.0 > > compatibly then the behavior of this class should be consistent with the > > pre-1.0 version. This will break 1.0 to 1.3 but anyone coding to a > post-1.0 > > version should not be using this deprecated class. > > > > > > > On Nov 28, 2017, at 5:22 PM, Jason Huynh <jhu...@pivotal.io> wrote: > > > > > > *With your proposoal 1.0 - 1.3 users would have modify their source > code > > on > > > the client and the server forthe function, correct?* > > > > > > If they start a new geode server 1.4+ and happened to extend > > > functionAdapter (it was deprecated in 1.0) then they would have to > > > recompile their client to not use functionAdapter. > > > > > > This should only affect users that extend FunctionAdapter and execute > > > functions by serializing them to the server from the client. If they > > > execute by id it should not run into this problem... > > > > > >> On Tue, Nov 28, 2017 at 5:20 PM Jason Huynh <jhu...@pivotal.io> > wrote: > > >> > > >> Dan, yeah, the suggested change in the stack overflow answer does work > > and > > >> I was able to put an if with the exact serialVersionUid before posting > > the > > >> proposal, but it is pretty hacky and may affect another class that > > somehow > > >> generated the same uid. I can make that change too but I'd prefer not > > to > > >> have to maintain it moving forward... > > >> > > >> > > >> > > >>> On Tue, Nov 28, 2017 at 5:09 PM Dan Smith <dsm...@pivotal.io> wrote: > > >>> > > >>> I agree I don't think we can get rid of FunctionAdapter until the > next > > >>> major version. > > >>> > > >>> I was thinking FunctionAdapter is rather widely used, but then I'm > > >>> surprised no one has hit this yet. > > >>> > > >>> All of the options kinda suck here - either pre 1.0 users have a > > >>> compatibility issue or 1.0-1.3 users do. With your proposoal 1.0 - > 1.3 > > >>> users would have modify their source code on the client and the > server > > for > > >>> the function, correct? > > >>> > > >>> If we got really fancy we could actually ignore the serialVersionUUID > > for > > >>> this class like this - https://stackoverflow.com/a/1816711/2813144. > > But > > >>> it's pretty messy. > > >>> > > >>> -Dan > > >>> > > >>> On Tue, Nov 28, 2017 at 1:59 PM, Alexander Murmann < > > amurm...@pivotal.io> > > >>> wrote: > > >>> > > >>>> Anil, I am not sure following. I think FunctionAdapter already is > > >>>> deprecated. Isn't it? Anthony is right though that we shouldn't > remove > > >>>> anything customer facing unless we are doing a major release. > > Otherwise > > >>> we > > >>>> are violating the contract provided by semantic versioning. > > >>>> > > >>>> On Tue, Nov 28, 2017 at 1:52 PM, Anilkumar Gingade < > > aging...@pivotal.io > > >>>> > > >>>> wrote: > > >>>> > > >>>>> I haven't seen many uses of FunctionAdapter; if its not used much, > I > > >>>> think > > >>>>> we should deprecate this... > > >>>>> > > >>>>> It only provided default implementation for few of the methods; > this > > >>>> could > > >>>>> be added in the docs/release notes to help application to move to > > >>>> function > > >>>>> implementation. > > >>>>> > > >>>>> -Anil. > > >>>>> > > >>>>> > > >>>>> On Tue, Nov 28, 2017 at 12:40 PM, Anthony Baker <aba...@pivotal.io > > > > >>>> wrote: > > >>>>> > > >>>>>> I think we should wait for a major release to remove API’s. If we > > >>>> broke > > >>>>> a > > >>>>>> public API, we should fix that IMO. > > >>>>>> > > >>>>>> Anthony > > >>>>>> > > >>>>>> > > >>>>>>> On Nov 28, 2017, at 11:40 AM, Patrick Rhomberg < > > >>> prhomb...@pivotal.io > > >>>>> > > >>>>>> wrote: > > >>>>>>> > > >>>>>>> +1 to removing a long-deprecated class from the Geode side. > > >>>>>>> > > >>>>>>> On Tue, Nov 28, 2017 at 8:04 AM, Bruce Schuchardt < > > >>>>>> bschucha...@pivotal.io> > > >>>>>>> wrote: > > >>>>>>> > > >>>>>>>> How about just getting rid of this class? After all it was > > >>> marked > > >>>> as > > >>>>>>>> being deprecated in 1.0. Pivotal could add a compatible > > >>>>> FunctionAdapter > > >>>>>>>> class in their GemFire builds to support these old clients. > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>>> On 11/27/17 10:18 AM, Jason Huynh wrote: > > >>>>>>>>> > > >>>>>>>>> This is a discussion for the fix to GEODE-4008: > > >>>>>>>>> InvalidClassException when deserializing FunctionAdapter from > > >>> pre > > >>>>> Geode > > >>>>>>>>> clients > > >>>>>>>>> > > >>>>>>>>> There was a change to deprecate FunctionAdapter in Geode > (before > > >>>>> 1.0), > > >>>>>> and > > >>>>>>>>> this also removed the method signatures in the class. This > > >>> caused > > >>>>> Java > > >>>>>> to > > >>>>>>>>> generate a new serialVersionUID to the class because one was > not > > >>>>>> assigned > > >>>>>>>>> previously. However we have clients pre Geode that when they > > >>>> attempt > > >>>>> to > > >>>>>>>>> execute a function by serializing the function across (not > > >>> using a > > >>>>>>>>> function > > >>>>>>>>> id), the FunctionAdapter class is unable to deserialize > > >>> properly. > > >>>>>>>>> > > >>>>>>>>> The proposed fix is to assign a serialVersionUID to the class > > >>> that > > >>>>>> matches > > >>>>>>>>> that of the pre Geode FunctionAdapter. This will cause any > Geode > > >>>>>> 1.0-1.3 > > >>>>>>>>> clients to now run into the error but the older clients would > > >>> work > > >>>>>> fine. > > >>>>>>>>> Because FunctionAdapter has been deprecated it should be easy > > >>>> enough > > >>>>>> for > > >>>>>>>>> Geode 1.0-1.3 users to change their custom classes to implement > > >>>>>> Function > > >>>>>>>>> directly and not use the deprecated FunctionAdapter class. > > >>>>>>>>> > > >>>>>>>>> Please let me know if there is a better solution or if there > are > > >>>>>> problems > > >>>>>>>>> with the proposed fix. > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> Thanks, > > >>>>>>>>> > > >>>>>>>>> -Jason > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>> > > >>>>>> > > >>>>> > > >>>> > > >>> > > >> > > >
