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]> 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]> 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. >
