+1 On Mon, Apr 3, 2017 at 2:33 AM, Aljoscha Krettek <[email protected]> wrote:
> +1 for making this consistent > > On Mon, Apr 3, 2017, at 10:20, Etienne Chauchot wrote: > > +1 > > > > Etienne > > > > > > Le 30/03/2017 à 20:48, Kenneth Knowles a écrit : > > > +1 for a different reason (also: now is the time to revisit names and > other > > > bits of the State API before it is too late :-) > > > > > > Folks may not have this catalog in their head. The classes / methods > are: > > > > > > ValueState / value(...) > > > BagState / bag(...) > > > SetState / set(...) > > > MapState / map(...) > > > AccumulatorCombiningState / combiningValue(...) > > > > > > I propose renaming this last one to CombiningState / combining(...) to > > > match here and also to match CombineFn. > > > > > > I propose renaming what is currently CombiningState to something like > > > GroupingState and probably just moving to runners-core (as a trivial > > > wrapper, since it won't be able to remain a superclass) since it is not > > > useful for users. > > > > > > Kenn > > > > > > On Thu, Mar 30, 2017 at 9:32 AM, Jean-Baptiste Onofré <[email protected] > > > > > wrote: > > > > > >> +0 > > >> > > >> as the StateSpec takes AccumularCombiningState, it's already "specify" > > >> indirectly. > > >> > > >> Regards > > >> JB > > >> > > >> > > >> On 03/30/2017 04:46 PM, Etienne Chauchot wrote: > > >> > > >>> Hi all, > > >>> > > >>> Just a 10 cents comment, but maybe a rename of public method > > >>> nevertheless... > > >>> > > >>> There are AccumulatorCombiningState and CombiningState interfaces, > the > > >>> first > > >>> extends the second > > >>> > > >>> The factory method StateSpecs.combiningValue returns a > > >>> StateSpec<Object, ** AccumulatorCombiningState<InputT, AccumT, > OutputT> > > >>> ** > > > >>> > > >>> The method name seems confusing for a user, who would search for a > factory > > >>> method returning a CombiningState (but there is none). > > >>> > > >>> Maybe StateSpecs.accumulatorCombiningValue would be a better name? > > >>> > > >>> The same goes for StateSpecs.*CombiningValue methods that all return > > >>> AccumulatorCombiningState but maybe the name would be too long. > > >>> > > >>> WDYT? > > >>> > > >>> Etienne > > >>> > > >>> > > >> -- > > >> Jean-Baptiste Onofré > > >> [email protected] > > >> http://blog.nanthrax.net > > >> Talend - http://www.talend.com > > >> > > >
