spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Davidson <ilike...@gmail.com>
Subject Re: Proposal: Clarifying minor points of Scala style
Date Tue, 11 Feb 2014 05:00:18 GMT
Alright, makes sense -- consistency is more important than special casing
for possible readability benefits. That is one of the main points behind a
style guide after all. I switch my vote for (1) to Shivaram's proposal as
well.


On Mon, Feb 10, 2014 at 4:40 PM, Evan Chan <ev@ooyala.com> wrote:

> +1 to the proposal.
>
> On Mon, Feb 10, 2014 at 2:56 PM, Michael Armbrust
> <michael@databricks.com> wrote:
> > +1 to Shivaram's proposal.  I think we should try to avoid functions with
> > many args as much as possible so having a high vertical cost here isn't
> the
> > worst thing.  I also like the visual consistency.
> >
> > FWIW, (based on a cursory inspection) in the scala compiler they don't
> seem
> > to ever orphan the return type from the closing parenthese.  It seems
> there
> > are two main accepted styles:
> >
> >     def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree,
> syncBody:
> > List[Tree],
> >                       stats: List[Tree], retVal: Tree): Tree = {
> >
> > and
> >
> >     def tryToSetIfExists(
> >       cmd: String,
> >       args: List[String],
> >       setter: (Setting) => (List[String] => Option[List[String]])
> >     ): Option[List[String]] =
> >
> >
> > On Mon, Feb 10, 2014 at 2:36 PM, Shivaram Venkataraman <
> > shivaram@eecs.berkeley.edu> wrote:
> >
> >> Yeah that was my proposal - Essentially we can just have two styles:
> >> The entire function + parameterList + return type fits in one line or
> >> when it doesn't we wrap parameters into lines.
> >> I agree that it makes the code a more verbose, but it'll make code
> >> style more consistent.
> >>
> >> Shivaram
> >>
> >> On Mon, Feb 10, 2014 at 2:13 PM, Aaron Davidson <ilikerps@gmail.com>
> >> wrote:
> >> > Shivaram, is your recommendation to wrap the parameter list even if it
> >> fits,
> >> > but just the return type doesn't? Personally, I think the cost of
> moving
> >> > from a single-line parameter list to an n-ine list is pretty high, as
> it
> >> > takes up a lot more space. I am even in favor of allowing a parameter
> >> list
> >> > to overflow into a second line (but not a third) instead of spreading
> >> them
> >> > out, if it's a private helper method (where the parameters are
> probably
> >> not
> >> > as important as the implementation, unlike a public API).
> >> >
> >> >
> >> > On Mon, Feb 10, 2014 at 1:42 PM, Shivaram Venkataraman
> >> > <shivaram@eecs.berkeley.edu> wrote:
> >> >>
> >> >> For the 1st case wouldn't it be better to just wrap the parameters
to
> >> >> the next line as we do in other cases ? For example
> >> >>
> >> >> def longMethodName(
> >> >>     param1,
> >> >>     param2, ...) : Long = {
> >> >> }
> >> >>
> >> >> Are there a lot functions which use the old format ? Can we just
> stick
> >> >> to the above for new functions ?
> >> >>
> >> >> Thanks
> >> >> Shivaram
> >> >>
> >> >> On Mon, Feb 10, 2014 at 11:33 AM, Reynold Xin <rxin@databricks.com>
> >> wrote:
> >> >> > +1 on both
> >> >> >
> >> >> >
> >> >> > On Mon, Feb 10, 2014 at 1:34 AM, Aaron Davidson <
> ilikerps@gmail.com>
> >> >> > wrote:
> >> >> >
> >> >> >> There are a few bits of the Scala style that are underspecified
by
> >> >> >> both the Scala
> >> >> >> style guide <http://docs.scala-lang.org/style/> and
our own
> >> >> >> supplemental
> >> >> >> notes<
> >> >> >>
> >> >> >>
> >>
> https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide>.
> >> >> >> Often, this leads to inconsistent formatting within the codebase,
> so
> >> >> >> I'd
> >> >> >> like to propose some general guidelines which we can add to
the
> wiki
> >> >> >> and
> >> >> >> use in the future:
> >> >> >>
> >> >> >> 1) Line-wrapped method return type is indented with two spaces:
> >> >> >> def longMethodName(... long param list ...)
> >> >> >>   : Long = {
> >> >> >>   2
> >> >> >> }
> >> >> >>
> >> >> >> *Justification: *I think this is the most commonly used style
in
> >> Spark
> >> >> >> today. It's also similar to the "extends" style used in classes,
> with
> >> >> >> the
> >> >> >> same justification: it is visually distinguished from the
> 4-indented
> >> >> >> parameter list.
> >> >> >>
> >> >> >> 2) URLs and code examples in comments should not be line-wrapped.
> >> >> >> Here<
> >> >> >>
> >> >> >>
> >>
> https://github.com/apache/incubator-spark/pull/557/files#diff-c338f10f3567d4c1d7fec4bf9e2677e1L29
> >> >> >> >is
> >> >> >> an example of the latter.
> >> >> >>
> >> >> >> *Justification*: Line-wrapping can cause confusion when trying
to
> >> >> >> copy-paste a URL or command. Can additionally cause IDE issues
or,
> >> >> >> avoidably, Javadoc issues.
> >> >> >>
> >> >> >> Any thoughts on these, or additional style issues not explicitly
> >> >> >> covered in
> >> >> >> either the Scala style guide or Spark wiki?
> >> >> >>
> >> >
> >> >
> >>
>
>
>
> --
> --
> Evan Chan
> Staff Engineer
> ev@ooyala.com  |
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message