I haven’t heard a disagreement. Maybe someone should put the requirements on the CMS.
This long email includes proposed: * For Scala follow: http://docs.scala-lang.org/style/ * We are relaxing the required use of infix for *all* appropriate method calls and allowing “dot” notation. Spark goes further to say you must not use infix unless it’s an operator method. * Line lengths not to exceed 120 chars—Spark limits to 100 This is so close to Spark’s guidelines here: https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide that maybe we should just reference them. On Jan 25, 2015, at 9:21 AM, Andrew Musselman <[email protected]> wrote: Of course we should be attentive to style, agree review of style should be a prerequisite to sign off on a PR or commit: On Saturday, January 24, 2015, Dmitriy Lyubimov <[email protected]> wrote: > Pat, > > I have no (particular) clue what you are talking about. All I am asking is > for committers not to let in commits containing 170 character long lines if > possible please. Or zigzagy shaped comments. > > There are people on this project that used to ask for no trailing > whitespaces that one can't even see unless review board highlights them. Or > that style plugins produce no new warnings. I am not one of them. I just > ask to be able to look at the stuff without turning my screen inside out. > As it has been pointed before by other people in style discussions, too > obvious style sloppiness is not adding to code credibility. Hope that much > is still agreeable. > > Other than that, I don't object any reasonably adopted style guidelines. > > Although as I mentioned, as a spark contributor, I prefer not to deviate > from a single set of rules already set for spark code base, although have > no strong opinion about it as I don't think anybody else here hacks both at > the same time to care enough. > > I dont intend to sound preachy on the subject but please note that there is > difference between committer and contributor roles. Committer is > specifically charged with style and coherence checks, and doing general > code vetting for quality regardless of point of origin of such code. The > fact that we have lines longer than 120 characters in the code base > directly means lapse in committer duties. Using java style coding in Scala > is also one thing that should trigger committee red flags. > > Imo we should try to do it better, shouldn't we? Restore quality peer > reviews practice? > The comments are formatted using IDE defaults, I agree they can be cleaned > up. > > The Scala docs here: > http://docs.scala-lang.org/style/method-invocation.html > are the usual source of guidance. There have been a couple exceptions to > these guidelines including: > 1) We (in much of the mahout scala code I’ve seen) don’t seem to follow the > "no dot” infix style recommended in the guide > http://docs.scala-lang.org/style/method-invocation.html There was a > discussion of this some time ago and seemed be consensus for a more > java-like form. > > As to removing org.apache.mahout.common.Pair this seems like a good idea in > light of the recent refactoring. I took it from a quick back of the > envelope function that Sebastian wrote over a year ago when it was a > non-issue. I’ll look at that the next time I’m in the code. I assume this > would remove the need for Pair being in the module replacing mr-leagacy for > Scala? > > On Jan 24, 2015, at 5:10 AM, Gokhan Capan <[email protected] > <javascript:;>> wrote: > > +1 for favoring native scala types. > > I think in terms of Scala code, we need a clear style standards definition > to adhere to. > > > Gokhan > > On Fri, Jan 23, 2015 at 9:38 PM, Dmitriy Lyubimov <[email protected] > <javascript:;>> wrote: > >> in TextDelimitedReaderWriter.scala: >> >> =========================== >> val itemList: >> collection.mutable.MutableList[org.apache.mahout.common.Pair[Integer, >> Double]] = new >> collection.mutable.MutableList[org.apache.mahout.common.Pair[Integer, >> Double]] >> for (ve <- itemVector.nonZeroes) { >> val item: org.apache.mahout.common.Pair[Integer, Double] = new >> org.apache.mahout.common.Pair[Integer, Double](ve.index, ve.get) >> itemList += item >> } >> ================================ >> >> (1) why scala code attempts to use commons.pair? What was wrong about >> native Tuple type of scala? (I am trying to clean out mrlegacy > dependencies >> from spark module). >> >> (2) why it is so horribly styled (even for me)? comments are misaligned, >> the lines routinely exceed 120 characters? >> >> Can these problems please be addressed? in particular, stuff like >> o.a.m.common.Pair? And why it is even signed off on in the first place by >> committers despite of clear style violations? >> >> thank you. >> >
