On Mon, Oct 24, 2016 at 10:12 AM, Kenneth Knowles <[email protected]> wrote: > The precedent that we use verbs has many exceptions. We have > ApproximateQuantiles, Values, Keys, WithTimestamps, and I would even > include Sum (at least when I read it).
True. > Historical note: the predilection towards verbs is from the Google Style > Guide for Java method names > <https://google.github.io/styleguide/javaguide.html#s5.2.3-method-names>, > which states "Method names are typically verbs or verb phrases". But even > in Google code there are lots of exceptions when it makes sense, like Guava's > Iterables.any(), Iterables.all(), Iterables.toArray(), the entire > Predicates module, etc. Just an aside; Beam isn't Google code. I suggest we > use our judgment rather than a policy. Yes, we should favor what flows well. Verbs often do, but... > I think "Distinct" is one of those exceptions. It is a standard widespread > name and also reads better as an adjective. I prefer it, but also don't > care strongly enough to change it or to change it back :-) > > If we must have a verb, I like it as-is more than MakeDistinct and > AvoidDuplicate. I much prefer "Distinct" to the other options forcing it to be verb-like (despite being the one to bring this up). My (weak) preference is to leave RemoveDuplicates with better documentation, but Distinct could be fine. > On Mon, Oct 24, 2016 at 9:46 AM Jesse Anderson <[email protected]> > wrote: > >> My original thought for this change was that Crunch uses the class name >> Distinct. SQL also uses the keyword distinct. >> >> Maybe the rule should be changed to adjectives or verbs depending on the >> context. >> >> Using a verb to describe this class really doesn't connote what the class >> does as succinctly as the adjective. >> >> On Mon, Oct 24, 2016 at 9:40 AM Neelesh Salian <[email protected]> >> wrote: >> >> > Hello, >> > >> > First of all, thank you to Daniel, Robert and Jesse for their review on >> > this: https://issues.apache.org/jira/browse/BEAM-239 >> > >> > A point that came up was using verbs explicitly for Transforms. >> > Here is the PR: https://github.com/apache/incubator-beam/pull/1164 >> > >> > Posting it to help understand if we have a consensus for it and if yes, >> we >> > could perhaps document it for future changes. >> > >> > Thank you. >> > >> > -- >> > Neelesh Srinivas Salian >> > Engineer >> > >>
