On Mon, May 22, 2017 at 10:47 AM, Robert Bradshaw < [email protected]> wrote: > > For a simple rename like this, I would at least strongly encourage > fixing both for those that are capable. >
Agreed. My response suffered from premature generalization, wandering off into "policy"-esque topics. Kenn > > On Sun, May 21, 2017 at 11:45 AM, Dan Halperin > <[email protected]> > > wrote: > > > >> I think this is an unrealistic request -- Python and Java workflows are > >> completely different, and Python developer documentation is especially > >> abysmal. > >> > >> (E.g., I had to have Robert sit with me to get the Python SDK to work at > >> all on my developer machine, and even then I gave up and chmod-ed my > >> machine-wide Python repos to be world-writable to get it to work.) > >> > >> On Fri, May 19, 2017 at 4:50 PM, Ahmet Altay <[email protected]> > >> wrote: > >> > >> > I mentioned this in the PR, I believe it is worth repeating here. > >> > > >> > It is important to keep the API consistent between Python and Java. It > >> > would help a lot, if changes are applied to both SDKs at the same > time. > >> If > >> > that is not possible, an easier alternative would be to file a JIRA > issue > >> > so that the work could be tracked in the other SDK. > >> > > >> > Ahmet > >> > > >> > On Fri, May 19, 2017 at 4:22 PM, Robert Bradshaw < > >> > [email protected]> wrote: > >> > > >> > > I see this was implemented. Do we have a policy/guideline for when a > >> > > name is "bad enough" to merit renaming (and keeping a duplicate, > >> > > deprecated member around for a year or more). > >> > > > >> > > On Mon, May 15, 2017 at 9:25 AM, Robert Bradshaw < > [email protected]> > >> > > wrote: > >> > > > On Sun, May 14, 2017 at 3:36 PM, Ben Chambers > >> > > <[email protected]> > >> > > > wrote: > >> > > >> > >> > > >> Exposing the CombineFn is necessary for use with composed > combine or > >> > > >> combining value state. There may be other reasons we made this > >> > visible, > >> > > >> but > >> > > >> these continue to justify it. > >> > > > > >> > > > > >> > > > These are the CompareFns, not the CombineFns. > >> > > > > >> > > > It'd be nicer to use the Guava and/or Java8 natural ordering > >> > comparables, > >> > > > but they don't promise serializable. > >> > > > > >> > > > I agree the naming is unfortunate, but I don't know that it's bad > >> > enough > >> > > to > >> > > > introduce a new name and have duplication and deprecation in the > API. > >> > It > >> > > > also goes deeper than this; Top.of(...) gives elements in > >> *decreasing* > >> > > order > >> > > > while List.sort(...) gives elements in *increasing* order so > using a > >> > > > comparator in one will always produce the opposite effect of > using a > >> > > > comparator in the other. > >> > > > > >> > > >> > >> > > >> On Sun, May 14, 2017, 1:00 PM Reuven Lax > <[email protected]> > >> > > wrote: > >> > > >> > >> > > >> > I believe the reason why this is called Top.largest, is that > >> > > originally > >> > > >> > it > >> > > >> > was simply the comparator used by Top.largest - i.e. and > >> > > implementation > >> > > >> > detail. At some point it was made public and used by other > >> > transforms > >> > > - > >> > > >> > maybe making an implementation detail a public class was the > real > >> > > >> > mistake? > >> > > >> > > >> > > >> > On Sun, May 14, 2017 at 11:45 AM, Davor Bonaci < > [email protected]> > >> > > wrote: > >> > > >> > > >> > > >> > > I agree this is an unfortunate name. > >> > > >> > > > >> > > >> > > Tangential: can we rename APIs now that the first stable > release > >> > is > >> > > >> > nearly > >> > > >> > > done? > >> > > >> > > Of course -- the "rename" can be done by introducing a new > API, > >> > and > >> > > >> > > deprecating, but not removing, the old one. Then, once we > decide > >> > to > >> > > >> > > move > >> > > >> > to > >> > > >> > > the next major release, the deprecated API can be removed. > >> > > >> > > > >> > > >> > > I think we should probably do the "rename" at some point, but > >> I'd > >> > > >> > > leave > >> > > >> > the > >> > > >> > > final call to the wider consensus. > >> > > >> > > > >> > > >> > > On Sat, May 13, 2017 at 5:16 PM, Wesley Tanaka > >> > > >> > > <[email protected] > >> > > >> > > > >> > > >> > > wrote: > >> > > >> > > > >> > > >> > > > Using Top.Largest to sort a list of {2,1,3} produces > {1,2,3}. > >> > > This > >> > > >> > > > matches the javadoc for the class, but seems > counter-intuitive > >> > -- > >> > > >> > > > one > >> > > >> > > might > >> > > >> > > > expect that a Comparator called Largest would give largest > >> items > >> > > >> > > > first. > >> > > >> > > > I'm wondering if renaming the classes to Natural / Reversed > >> > would > >> > > >> > better > >> > > >> > > > match their behavior? > >> > > >> > > > > >> > > >> > > > --- > >> > > >> > > > Wesley Tanaka > >> > > >> > > > https://wtanaka.com/ > >> > > >> > > > >> > > >> > > >> > > > > >> > > > > >> > > > >> > > >> >
