As a trial, it could be configured on Jenkins to run the job as a non-critical step, where failure wouldn't constitute a build failure, and assess whether it's useful by trying it out. Scalafmt only works for Scala sources, but as Cody says, there are other utilities for Java. From my experience, scalafmt works quite well (very few cases where running twice is not idempotent).
El vie., 23 nov. 2018 a las 2:32, Matei Zaharia (<matei.zaha...@gmail.com>) escribió: > Can we start by just recommending to contributors that they do this > manually? Then if it seems to work fine, we can try to automate it. > > > On Nov 22, 2018, at 4:40 PM, Cody Koeninger <c...@koeninger.org> wrote: > > > > I believe scalafmt only works on scala sources. There are a few > > plugins for formatting java sources, but I'm less familiar with them. > > On Thu, Nov 22, 2018 at 11:39 AM Mridul Muralidharan <mri...@gmail.com> > wrote: > >> > >> Is this handling only scala or java as well ? > >> > >> Regards, > >> Mridul > >> > >> On Thu, Nov 22, 2018 at 9:11 AM Cody Koeninger <c...@koeninger.org> > wrote: > >>> > >>> Plugin invocation is ./build/mvn mvn-scalafmt_2.12:format > >>> > >>> It takes about 5 seconds, and errors out on the first different file > >>> that doesn't match formatting. > >>> > >>> I made a shell wrapper so that contributors can just run > >>> > >>> ./dev/scalafmt > >>> > >>> to actually format in place the files that have changed (or pass > >>> through commandline args if they want to do something different) > >>> > >>> On Wed, Nov 21, 2018 at 3:36 PM Sean Owen <sro...@gmail.com> wrote: > >>>> > >>>> I know the PR builder runs SBT, but I presume this would just be a > >>>> separate mvn job that runs. If it doesn't take long and only checks > >>>> the right diff, seems worth a shot. What's the invocation that Shane > >>>> could add (after this change goes in) > >>>> On Wed, Nov 21, 2018 at 3:27 PM Cody Koeninger <c...@koeninger.org> > wrote: > >>>>> > >>>>> There's a mvn plugin (sbt as well, but it requires sbt 1.0+) so it > >>>>> should be runnable from the PR builder > >>>>> > >>>>> Super basic example with a minimal config that's close to current > >>>>> style guide here: > >>>>> > >>>>> https://github.com/apache/spark/compare/master...koeninger:scalafmt > >>>>> > >>>>> I imagine tracking down the corner cases in the config, especially > >>>>> around interactions with scalastyle, may take a bit of work. Happy > to > >>>>> do it, but not if there's significant concern about style related > >>>>> changes in PRs. > >>>>> On Wed, Nov 21, 2018 at 2:42 PM Sean Owen <sro...@gmail.com> wrote: > >>>>>> > >>>>>> Yeah fair, maybe mostly consistent in broad strokes but not in the > details. > >>>>>> Is this something that can be just run in the PR builder? if the > rules > >>>>>> are simple and not too hard to maintain, seems like a win. > >>>>>> On Wed, Nov 21, 2018 at 2:26 PM Cody Koeninger <c...@koeninger.org> > wrote: > >>>>>>> > >>>>>>> Definitely not suggesting a mass reformat, just on a per-PR basis. > >>>>>>> > >>>>>>> scalafmt --diff will reformat only the files that differ from git > head > >>>>>>> scalafmt --test --diff won't modify files, just throw an exception > if > >>>>>>> they don't match format > >>>>>>> > >>>>>>> I don't think code is consistently formatted now. > >>>>>>> I tried scalafmt on the most recent PR I looked at, and it caught > >>>>>>> stuff as basic as newlines before curly brace in existing code. > >>>>>>> I've had different reviewers for PRs that were literal backports or > >>>>>>> cut & paste of each other come up with different formatting nits. > >>>>>>> > >>>>>>> > >>>>>>> On Wed, Nov 21, 2018 at 12:03 PM Sean Owen <sro...@gmail.com> > wrote: > >>>>>>>> > >>>>>>>> I think reformatting the whole code base might be too much. If > there > >>>>>>>> are some more targeted cleanups, sure. We do have some links to > style > >>>>>>>> guides buried somewhere in the docs, although the conventions are > >>>>>>>> pretty industry standard. > >>>>>>>> > >>>>>>>> I *think* the code is pretty consistently formatted now, and would > >>>>>>>> expect contributors to follow formatting they see, so ideally the > >>>>>>>> surrounding code alone is enough to give people guidance. In > practice, > >>>>>>>> we're always going to have people format differently no matter > what I > >>>>>>>> think so it's inevitable. > >>>>>>>> > >>>>>>>> Is there a way to just check style on PR changes? that's fine. > >>>>>>>> On Wed, Nov 21, 2018 at 11:40 AM Cody Koeninger < > c...@koeninger.org> wrote: > >>>>>>>>> > >>>>>>>>> Is there any appetite for revisiting automating formatting? > >>>>>>>>> > >>>>>>>>> I know over the years various people have expressed opposition > to it > >>>>>>>>> as unnecessary churn in diffs, but having every new contributor > >>>>>>>>> greeted with "nit: 4 space indentation for argument lists" isn't > very > >>>>>>>>> welcoming. > >>>>>>>>> > >>>>>>>>> > --------------------------------------------------------------------- > >>>>>>>>> To unsubscribe e-mail: dev-unsubscr...@spark.apache.org > >>>>>>>>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe e-mail: dev-unsubscr...@spark.apache.org > >>> > > > > --------------------------------------------------------------------- > > To unsubscribe e-mail: dev-unsubscr...@spark.apache.org > > > > > --------------------------------------------------------------------- > To unsubscribe e-mail: dev-unsubscr...@spark.apache.org > >
