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 ;-)). >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >>
