By the way, I also don't see the benefit of doing the transition piece by piece.
On Mon, 27 Feb 2017 at 22:21 Dawid Wysakowicz <wysakowicz.da...@gmail.com> wrote: > I agree with adopting a custom codestyle/checkstyle for flink, but as I > understood correctly most people agree there is no point of providing an > unenforced code style. > > 2017-02-27 22:04 GMT+01:00 Greg Hogan <c...@greghogan.com>: > > > There was also … > > > > - create a flink style (for example, leaving indentation as +1 tab rather > > than +2 spaces as in google's style) > > > > - create a flink style but optional and unenforced (but recommended for > > new contributions) > > > > Flink currently has a reasonably consistent code style. I expect that > > adopting a radically different code style module-by-module will also > result > > in contributions with a mix of old an new styles. If we’re not willing to > > re-style flink-core today, under what circumstances will this change? > With > > a punctuated refactoring there would be a singular event for developers > to > > remember (as with the initial commits). > > > > Greg > > > > > > > On Feb 27, 2017, at 3:40 PM, Dawid Wysakowicz < > > wysakowicz.da...@gmail.com> wrote: > > > > > > So to sum up all the comments so far we have two alternatives. > > > We either: > > > 1) introduce unified checkstyle (with enforcing) and corresponding code > > > style, both based on some established ones like google code style for > > java > > > [1] <https://github.com/google/google-java-format> and scalastyle for > > scala > > > [2] <http://www.scalastyle.org/> . We would introduce it module by > > module > > > for a longer period of time > > > or > > > 2) leave it as it is, and end this discussion for a longer (possibly > > > infinite :) ) period of time > > > > > > Not sure how we should proceed with the decision on it. Is it possible > to > > > do some voting or so? > > > > > > 2017-02-27 20:54 GMT+01:00 Stavros Kontopoulos < > st.kontopou...@gmail.com > > >: > > > > > >> +1 to provide and enforcing a unified code style for both java and > > scala. > > >> Unification should apply when it makes sense like comments though. > > >> > > >> Eventually code base should be re-factored. I would vote for the one > at > > a > > >> time module fix apporoach. > > >> Style guide should be part of any PR review. > > >> > > >> We could also have a look at the spark style guide: > > >> https://github.com/databricks/scala-style-guide > > >> > > >> The style code and general guidelines help keep code more readable and > > keep > > >> things simple > > >> with many contributors and different styles of code writing + language > > >> features. > > >> > > >> > > >> On Mon, Feb 27, 2017 at 8:01 PM, Stephan Ewen <se...@apache.org> > wrote: > > >> > > >>> I agree, reformatting 90% of the code base is tough. > > >>> > > >>> There are two main issues: > > >>> (1) Incompatible merges. This is hard, especially for the folks that > > >> have > > >>> to merge the pull requests ;-) > > >>> > > >>> (2) Author history: This is less of an issue, I think. "git log > > >>> <filename>" and "git show <revision> -- <filename>" will still work > and > > >> one > > >>> may have to go one commit back to find out why something was changed > > >>> > > >>> > > >>> What I could image is to do this incrementally. Define the code style > > in > > >>> "flink-parent" but do not activate it. > > >>> Then start with some projects (new projects, plus some others): > > >>> merge/reject PRs, reformat, activate code style. > > >>> > > >>> Piece by piece. This is realistically going to take a long time until > > it > > >> is > > >>> pulled through all components, but that's okay, I guess. > > >>> > > >>> Stephan > > >>> > > >>> > > >>> On Mon, Feb 27, 2017 at 1:53 PM, Aljoscha Krettek < > aljos...@apache.org > > > > > >>> wrote: > > >>> > > >>>> Just for a bit of context, this is the output of running cloc on the > > >>> Flink > > >>>> codebase: > > >>>> ------------------------------------------------------------ > > >>>> ----------------------- > > >>>> Language files blank comment > > >>>> code > > >>>> ------------------------------------------------------------ > > >>>> ----------------------- > > >>>> Java 4609 126825 185428 > > >>>> 519096 > > >>>> > > >>>> => 704,524 lines of code + comments/javadoc > > >>>> > > >>>> When I apply the google style to the Flink code base using > > >>>> https://github.com/google/google-java-format I get these commit > > >>>> statistics: > > >>>> > > >>>> 4577 files changed, 647645 insertions(+), 622663 deletions(-) > > >>>> > > >>>> That is, a change to the Google Code Style would touch roughly over > > 90% > > >>> of > > >>>> all code/comment lines. > > >>>> > > >>>> I would like to have a well defined code style, such as the Google > > Code > > >>>> style, that has nice tooling and support but I don't think we will > > ever > > >>>> convince enough people to do this kind of massive change. Even I > think > > >>> it's > > >>>> a bit crazy to change 90% of the code base in one commit. > > >>>> > > >>>> On Mon, 27 Feb 2017 at 11:10 Till Rohrmann <trohrm...@apache.org> > > >> wrote: > > >>>> > > >>>>> No, I think that's exactly what people mean when saying "losing the > > >>>> commit > > >>>>> history". With the reformatting you would have to go manually > through > > >>> all > > >>>>> past commits until you find the commit which changed a given line > > >>> before > > >>>>> the reformatting. > > >>>>> > > >>>>> Cheers, > > >>>>> Till > > >>>>> > > >>>>> On Sun, Feb 26, 2017 at 6:32 PM, Alexander Alexandrov < > > >>>>> alexander.s.alexand...@gmail.com> wrote: > > >>>>> > > >>>>>> Just to clarify - by "losing the commit history" you actually mean > > >>>>> "losing > > >>>>>> the ability to annotate each line in a file with its last commit", > > >>>> right? > > >>>>>> > > >>>>>> Or is there some other sense in which something is lost after > > >>> applying > > >>>>> bulk > > >>>>>> re-format? > > >>>>>> > > >>>>>> Cheers, > > >>>>>> A. > > >>>>>> > > >>>>>> On Sat, Feb 25, 2017 at 7:10 AM Henry Saputra < > > >>> henry.sapu...@gmail.com > > >>>>> > > >>>>>> wrote: > > >>>>>> > > >>>>>>> Just want to clarify what unify code style here. > > >>>>>>> > > >>>>>>> Is the intention to have IDE and Maven plugins to have the same > > >>> check > > >>>>>> style > > >>>>>>> rules? > > >>>>>>> > > >>>>>>> Or are we talking about having ONE code style for both Java and > > >>>> Scala? > > >>>>>>> > > >>>>>>> - Henry > > >>>>>>> > > >>>>>>> On Fri, Feb 24, 2017 at 8:08 AM, Greg Hogan <c...@greghogan.com> > > >>>>> wrote: > > >>>>>>> > > >>>>>>>> I agree wholeheartedly with Ufuk. We cannot reformat the > > >>> codebase, > > >>>>>> cannot > > >>>>>>>> pause while flushing the PR queue, and won't find a consensus > > >>> code > > >>>>>> style. > > >>>>>>>> > > >>>>>>>> I think we can create a baseline code style for new and > > >> existing > > >>>>>>>> contributors for which reformatting on changed files will be > > >>>>> acceptable > > >>>>>>> for > > >>>>>>>> PR reviews. > > >>>>>>>> > > >>>>>>>> On Fri, Feb 24, 2017 at 5:01 AM, Dawid Wysakowicz < > > >>>>>>>> wysakowicz.da...@gmail.com> wrote: > > >>>>>>>> > > >>>>>>>>> The problem with code style when it is not enforced is that > > >> it > > >>>> will > > >>>>>> be > > >>>>>>> a > > >>>>>>>>> matter of luck to what parts of files / new files will it be > > >>>>> applied. > > >>>>>>>> When > > >>>>>>>>> the code style is not applied to whole file, it is pretty > > >> much > > >>>>>> useless > > >>>>>>>>> anyway. You would need to manually select just the fragments > > >>> one > > >>>> is > > >>>>>>>>> changing. The benefits of having code style and enforcing it > > >> I > > >>>> see > > >>>>>> are: > > >>>>>>>>> - being able to apply autoformatter, which speeds up writing > > >>>> code > > >>>>>>>>> - it would make reviewing PRs easier as e.g. there would be > > >>> line > > >>>>>>> length > > >>>>>>>>> limit applied etc. which will make line breaking more reader > > >>>>>> friendly. > > >>>>>>>>> > > >>>>>>>>> Though I think if a consensus is not reachable it would be > > >> good > > >>>> to > > >>>>>> once > > >>>>>>>> and > > >>>>>>>>> forever decide that we don't want a code style and > > >> checkstyle. > > >>>>>>>>> > > >>>>>>>>> 2017-02-24 10:51 GMT+01:00 Ufuk Celebi <u...@apache.org>: > > >>>>>>>>> > > >>>>>>>>>> On Fri, Feb 24, 2017 at 10:46 AM, Fabian Hueske < > > >>>>> fhue...@gmail.com > > >>>>>>> > > >>>>>>>>> wrote: > > >>>>>>>>>>> I agree with Till that encouraging a code style without > > >>>>> enforcing > > >>>>>>> it > > >>>>>>>>> does > > >>>>>>>>>>> not make a lot of sense. > > >>>>>>>>>>> If we enforce it, we need to touch all files and PRs. > > >>>>>>>>>> > > >>>>>>>>>> I think it makes sense for new contributors to have a > > >>> starting > > >>>>>> point > > >>>>>>>>>> without enforcing anything (I do agree that we are past the > > >>>> point > > >>>>>> to > > >>>>>>>>>> reach consensus on a style and enforcement ;-)). > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>>> > > >>>>>> > > >>>>> > > >>>> > > >>> > > >> > > > > >
