Mailing-List: contact dev-help@flink.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@flink.apache.org
MIME-Version: 1.0
In-Reply-To: <E6472FFF-B079-4887-A284-4EB5968F63F4@163.com>
References: <CA+YaHHUiOmLk7vtTvQFMaw1UH=uOOo5=9QfUTbN71Et7nq4Q8Q@mail.gmail.com>
 <CACt=kDt8XQRP1PZa9TZ5kv5kfMYGT9Rk-dnNXHXW0dYnpxmKRQ@mail.gmail.com>
 <CAKiyyaF-vMfN_6f6r96-CFZqzw817ypB3yBewpawxH6Wa0qCeQ@mail.gmail.com>
 <CAC27z=NCnnvP2zExkR_cCGvinqP-wte_PO0BW8srOC0v2JuiXQ@mail.gmail.com>
 <58AD70DF.7010001@apache.org> <CA+YaHHXDBhQsFRL7tLmxfswbPZF7FpUQQpjqoc8R1rhnD+Xexw@mail.gmail.com>
 <CA+3U0ZnPXTeA4Hv7zXNZVVmRh-xfhRhvwjStEk5ntLv2N9-viA@mail.gmail.com>
 <58ADB7A7.7010308@apache.org> <CA+YaHHXo8z-dUm96R-wuheNzhsT2Y1auvTfyNi92PVmP_de9Jg@mail.gmail.com>
 <CANMXwW0VeGUHTF-THr3w8u92CV+bNv3REgf3R=3iLRgp-ZrV=A@mail.gmail.com> <E6472FFF-B079-4887-A284-4EB5968F63F4@163.com>
From: Fabian Hueske <fhueske@gmail.com>
Date: Fri, 24 Feb 2017 10:46:54 +0100
Message-ID: <CAAdrtT3_JSWrjWm9Fyp+ZshsKTADvKDc8_KWyM_CnUCxyFdELA@mail.gmail.com>
Subject: Re: [DISCUSS] Code style / checkstyle
To: "dev@flink.apache.org" <dev@flink.apache.org>
Content-Type: multipart/alternative; boundary=001a11409ca8f176260549439d44
archived-at: Fri, 24 Feb 2017 09:47:31 -0000

--001a11409ca8f176260549439d44
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

We have discussed this issue several times in the past and never got around
to do it.

Although I agree that it would be nice to have a stricter code style, I
don't think we should introduce a code style.
IMO, at this point the costs (losing the history, PR hassle, etc.) would be
too high compared to the benefits (that I am aware of).

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.

On the other hand, it's not clear to me how much we would gain with a code
style and what's the cost of not having one.

Best, Fabian


2017-02-23 17:52 GMT+01:00 Jinkui Shi <shijinkui666@163.com>:

> Thanks to discuss this problem again.
> 1. Google checkstyle is good for java.
> 2. scala check style is here [1]
> 3. We can make a Flink plan contain issues, one sub-issue one rule.
> Resolve this in short time.
>
> Current code style may be historical accumulate. If we don=E2=80=99t norm=
alize the
> code step by step, new code will also follow bad style without strict
> checking.
> We can adjust the exist code once one rule. But there will be lots of
> conflict in the not merged pull requests. Need all the pull request resol=
ve
> the conflict manually.
> Resolve long term trouble in short time:)
>
>
> [1]  http://www.scalastyle.org/ <http://www.scalastyle.org/>
> > On Feb 23, 2017, at 23:31, Aljoscha Krettek <aljoscha@apache.org> wrote=
:
> >
> > If we go for a codestyle/checkstyle I would suggest to use the Google
> > style. This already has checkstyle, IntelliJ style, Eclipse style and a
> > code formatting tool and is well established. However, some people will
> not
> > like this style. In general, I think we will never manage to find a sty=
le
> > that all people will like.
> >
> > On Wed, 22 Feb 2017 at 18:36 Dawid Wysakowicz <
> wysakowicz.dawid@gmail.com>
> > wrote:
> >
> >> So how about preparing a code style and corresponding checkstyle and
> >> enabling it gradually module by module whenever shepherd/commiter with
> >> expertise in a module will have time to introduce/check such a change?
> >> Maybe it will make the "snowball effect" happen?
> >> I agree there is no point in preparing code style/checkstyle until it
> will
> >> be introduced somewhere. I will be willing to work on the checkstyle i=
f
> >> some volunteering modules appear.
> >>
> >> 2017-02-22 17:09 GMT+01:00 Chesnay Schepler <chesnay@apache.org>:
> >>
> >>> For file where we don't enforce checkstyle things should work they wa=
y
> >>> they do right now.
> >>>
> >>> Turn off auto-formatting, and only format code that you touched and
> >> that's
> >>> it. For these
> >>> modification we will have to check them manually in the PRs as we do
> now.
> >>>
> >>>
> >>> On 22.02.2017 16:22, Greg Hogan wrote:
> >>>
> >>>> Will not the code style be applied on save to any user-modified file=
?
> So
> >>>> this will clutter PRs and overwrite history.
> >>>>
> >>>> On Wed, Feb 22, 2017 at 6:19 AM, Dawid Wysakowicz <
> >>>> wysakowicz.dawid@gmail.com> wrote:
> >>>>
> >>>> I also agree with Till and Chesnayl. Anyway as to "capture the curre=
nt
> >>>>> style" I have some doubts if this is possible, as it changes file t=
o
> >>>>> file.
> >>>>>
> >>>>> Chesnay's suggestion as to were enforce the checkstyle seems
> reasonable
> >>>>> to
> >>>>> me, but I am quite new to the community :).
> >>>>> Enabling checkstyle for particular packages is possible.
> >>>>>
> >>>>> 2017-02-22 12:07 GMT+01:00 Chesnay Schepler <chesnay@apache.org>:
> >>>>>
> >>>>> I agree with Till.
> >>>>>>
> >>>>>> I would propose enforcing checkstyle on a subset of the modules,
> >>>>>>
> >>>>> basically
> >>>>>
> >>>>>> those that are not
> >>>>>> flink-runtime, flink-java, flink-streaming-java. These are the one=
s
> >> imo
> >>>>>> where messing with the history
> >>>>>> can be detrimental; for the others it isn't really important imo.
> >>>>>> (Note that i excluded scala code since i don't know the state of
> >>>>>> checkstyle compliance there)
> >>>>>>
> >>>>>> For flink-runtime we could maybe (don't know if it is supported)
> >> enforce
> >>>>>> checkstyle for all classes
> >>>>>> under o.a.f.migration, so that at least the flip-6 code is covered=
.
> >>>>>>
> >>>>>> Similarly, enforcing checkstyle for all tests should be fine as
> well.
> >>>>>>
> >>>>>> Regards,
> >>>>>> Chesnay
> >>>>>>
> >>>>>>
> >>>>>> On 22.02.2017 11:48, Till Rohrmann wrote:
> >>>>>>
> >>>>>> I think that not enforcing a code style is as good as not having a=
ny
> >>>>>>>
> >>>>>> code
> >>>>>
> >>>>>> style to be honest. Having an IntelliJ or Eclipse profile is nice
> and
> >>>>>>>
> >>>>>> some
> >>>>>
> >>>>>> people will probably use it, but my gut feeling is that the majori=
ty
> >>>>>>>
> >>>>>> won't
> >>>>>
> >>>>>> notice it.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>> Till
> >>>>>>>
> >>>>>>> On Wed, Feb 22, 2017 at 11:15 AM, Ufuk Celebi <uce@apache.org>
> >> wrote:
> >>>>>>>
> >>>>>>> Kurt's proposal sounds reasonable.
> >>>>>>>
> >>>>>>>> What about the following:
> >>>>>>>> - We try to capture the current style in an code style
> configuration
> >>>>>>>> (for IntelliJ and maybe Eclipse)
> >>>>>>>> - We provide that on the website for contributors to download
> >>>>>>>> - We don't enforce it, but new contributions and changes are fre=
e
> to
> >>>>>>>> format with this style as changes happen
> >>>>>>>>
> >>>>>>>> Practically speaking, this should not change much except maybe t=
he
> >>>>>>>> import order or whitespace after certain keywords, etc.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Wed, Feb 22, 2017 at 4:48 AM, Kurt Young <ykt836@gmail.com>
> >> wrote:
> >>>>>>>>
> >>>>>>>> +1 to provide a unified code style for both java and scala.
> >>>>>>>>>
> >>>>>>>>> -1 to adjust all the existing code to the new style in one step=
.
> >> The
> >>>>>>>>>
> >>>>>>>>> code's
> >>>>>>>>
> >>>>>>>> history contains very helpful information which can help
> >>>>>>>>> develops to understand why these codes are added, which jira is
> >>>>>>>>>
> >>>>>>>> related.
> >>>>>
> >>>>>> This information is too valuable to lose. I think we can
> >>>>>>>>> do the reformat thing step by step, each time when the codes
> being
> >>>>>>>>>
> >>>>>>>>> changed,
> >>>>>>>>
> >>>>>>>> we can adopt them to the new style.
> >>>>>>>>> IMHO this is also the reason why the unified code style is
> >> important.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Best,
> >>>>>>>>> Kurt
> >>>>>>>>>
> >>>>>>>>> On Wed, Feb 22, 2017 at 5:50 AM, Dawid Wysakowicz <
> >>>>>>>>> wysakowicz.dawid@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>>> I would like to resurrect the discussing ([1]
> >>>>>>>>>> <http://apache-flink-mailing-list-archive.1008284.n3.
> >>>>>>>>>> nabble.com/Code-style-guideline-for-Scala-td7526.html>
> >>>>>>>>>> , [2]
> >>>>>>>>>> <http://apache-flink-mailing-list-archive.1008284.n3.
> >>>>>>>>>> nabble.com/Intellij-code-style-td11092.html>)
> >>>>>>>>>> about creating unified code style(that could be imported to at
> >> least
> >>>>>>>>>> IntelliJ and Eclipse) and corresponding stricter checkstyle
> rules.
> >>>>>>>>>>
> >>>>>>>>>> I know that the hardest part is to adjust the existing code to
> the
> >>>>>>>>>>
> >>>>>>>>> new
> >>>>>
> >>>>>> checkstyle rules. Do you believe it would be worth the effort? All
> >>>>>>>>>> suggestions are welcome.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>
> >>
>
>

--001a11409ca8f176260549439d44--