[DISCUSS] Adopting a Code Style and Quality Guide

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Adopting a Code Style and Quality Guide

Robert Metzger
I've converted the google document into a markdown-based pull request here:
https://github.com/apache/flink-web/pull/224

I didn't put a ton of effort into splitting the guide into multiple pages.
It would be nice to reach consensus on the exact split in the PR.

On Thu, Jun 27, 2019 at 10:48 PM Hugo Louro <[hidden email]> wrote:

> +1. Thanks for working on the guide. It's very thorough and a good resource
> to learn good practices from.
>
> I would like use this thread as a placeholder for a couple of topics that
> may be deserving of further discussion on different threads:
>   - What's the best way to keep track of checkstyle version updates. For
> instance, currently there is a PR
> <https://github.com/apache/flink/pull/8870> proposing checkstyle to be
> updated because version 8.12 is no longer supported
>  - When classes import shaded dependencies, it is not trivial for IntelliJ
> to download and associate sources and javadocs, which makes debugging and
> navigate the code base harder. I tried installing the version of the
> library using maven from the CLI, and associate the sources "manually" on
> IntelliJ, but it seems it does not work (perhaps IntelliJ issue). Does
> anyone know of a good solution? If so, we should added here
> <
> https://ci.apache.org/projects/flink/flink-docs-release-1.8/flinkDev/ide_setup.html#intellij-idea
> >.
> I can volunteer for that if you tell me how to do it :)
> - did the community evaluate naming test methods according to these
> conventions <https://stackoverflow.com/a/1594049> ?
>
> Thanks
>
> On Mon, Jun 24, 2019 at 11:38 AM Stephan Ewen <[hidden email]> wrote:
>
> > I think it makes sense to also start individual [DISCUSS] threads about
> > extensions and follow-ups.
> > Various suggestions came up, partly as comments in the doc, some as
> > questions in other threads.
> >
> > Examples:
> >   - use of null in return types versus Optional versus @Nullable/@Nonnull
> >   - initialization of collection sizes
> >   - logging
> >
> > I think these would be best discussed in separate threads each.
> > So, for contributors to whom these issues are dear, feel free to spawn
> > these additional threads.
> > (Bear in mind it is close to 1.9 feature freeze time, so please leave
> this
> > discussions a bit of time so that all community members have a chance to
> > participate)
> >
> >
> >
> > On Mon, Jun 24, 2019 at 7:51 PM Stephan Ewen <[hidden email]> wrote:
> >
> > > Thanks for the pointer David.
> > >
> > > I was not aware of this tool and I have no experience with its
> practical
> > > impact. For example I would be curious what the effect of this is for
> > > existing PRs, merge conflicts, etc.
> > >
> > > If you want, feel free to start another discuss thread on the
> > introduction
> > > of such a tool.
> > >
> > > On Sun, Jun 23, 2019 at 6:32 PM David Morávek <[hidden email]> wrote:
> > >
> > >> I love this kind of unification being pushed forward, the benefit it
> has
> > >> on
> > >> code reviews is enormous. Just my two cents, did you guys think about
> > >> adopting an automatic code formatter for java, the same way as Apache
> > Beam
> > >> did?
> > >>
> > >> It is super easy to use for contributors as they don't need to keep
> any
> > >> particular coding style in mind and they can only focus on
> functionality
> > >> they want to fix, and it's also great for reviewers, because they only
> > see
> > >> the important changes. This also eliminates need for any special
> editor
> > /
> > >> checkstyle configs as the code formatting is part of the build itself.
> > >>
> > >> The one Beam uses is https://github.com/diffplug/spotless with
> > >> GoogleJavaFormat, it may be worth to look into.
> > >>
> > >> Best,
> > >> David
> > >>
> > >> On Fri, Jun 21, 2019 at 4:40 PM Stephan Ewen <[hidden email]>
> wrote:
> > >>
> > >> > Thanks, everyone, for the positive feedback :-)
> > >> >
> > >> > @Robert - It probably makes sense to break this down into various
> > pages,
> > >> > like PR, general code style guide, Java, component specific guides,
> > >> > formats, etc.
> > >> >
> > >> > Best,
> > >> > Stephan
> > >> >
> > >> >
> > >> > On Fri, Jun 21, 2019 at 4:29 PM Robert Metzger <[hidden email]
> >
> > >> > wrote:
> > >> >
> > >> > > It seems that the discussion around this topic has settled.
> > >> > >
> > >> > > I'm going to turn the Google Doc into a markdown file (maybe also
> > >> > multiple,
> > >> > > I'll try out different things) and then open a pull request for
> the
> > >> Flink
> > >> > > website.
> > >> > > I'll post a link to the PR here once I'm done.
> > >> > >
> > >> > > On Fri, Jun 14, 2019 at 9:36 AM zhijiang <
> > [hidden email]
> > >> > > .invalid>
> > >> > > wrote:
> > >> > >
> > >> > > > Thanks for providing this useful guide for benefiting both
> > >> contributors
> > >> > > > and committers in consistency.
> > >> > > >
> > >> > > > I just reviewed and learned the whole doc which covers a lot of
> > >> > > > information. Wish it further categoried and put onto somewhere
> for
> > >> > easily
> > >> > > > traced future.
> > >> > > >
> > >> > > > Best,
> > >> > > > Zhijiang
> > >> > > >
> ------------------------------------------------------------------
> > >> > > > From:Robert Metzger <[hidden email]>
> > >> > > > Send Time:2019年6月14日(星期五) 14:24
> > >> > > > To:dev <[hidden email]>
> > >> > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality Guide
> > >> > > >
> > >> > > > Thanks a lot for putting this together!
> > >> > > >
> > >> > > > I'm in the process of reworking the "How to contribute" pages
> [1]
> > >> and
> > >> > I'm
> > >> > > > happy to add the guide to the Flink website, once the discussion
> > >> here
> > >> > is
> > >> > > > over.
> > >> > > >
> > >> > > > [1] https://github.com/apache/flink-web/pull/217
> > >> > > >
> > >> > > > On Fri, Jun 14, 2019 at 3:21 AM Kurt Young <[hidden email]>
> > >> wrote:
> > >> > > >
> > >> > > > > Big +1 and thanks for preparing this.
> > >> > > > >
> > >> > > > > I think wha't more important is making sure most all the
> > >> contributors
> > >> > > can
> > >> > > > > follow
> > >> > > > > the same guide, a clear document is definitely a great start.
> > >> > > Committers
> > >> > > > > can
> > >> > > > > first try to follow the guide by self, and spread the standard
> > >> during
> > >> > > > code
> > >> > > > > reviewing.
> > >> > > > >
> > >> > > > > Best,
> > >> > > > > Kurt
> > >> > > > >
> > >> > > > >
> > >> > > > > On Thu, Jun 13, 2019 at 8:28 PM Congxian Qiu <
> > >> [hidden email]
> > >> > >
> > >> > > > > wrote:
> > >> > > > >
> > >> > > > > > +1 for this, I think all contributors can benefit from this.
> > >> > > > > >
> > >> > > > > > Best,
> > >> > > > > > Congxian
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > Aljoscha Krettek <[hidden email]> 于2019年6月13日周四
> > 下午8:14写道:
> > >> > > > > >
> > >> > > > > > > +1 I think this is a very good effort and should put to
> rest
> > >> some
> > >> > > > > > > back-and-forth discussions on PRs and some differences in
> > >> “style”
> > >> > > > > between
> > >> > > > > > > committers. ;-)
> > >> > > > > > >
> > >> > > > > > > > On 13. Jun 2019, at 10:21, JingsongLee <
> > >> > [hidden email]
> > >> > > > > > .INVALID>
> > >> > > > > > > wrote:
> > >> > > > > > > >
> > >> > > > > > > > big +1, the content is very useful and enlightening.
> > >> > > > > > > > But it's really too long to look at.
> > >> > > > > > > > +1 for splitting it and expose it to contributors.
> > >> > > > > > > >
> > >> > > > > > > > Even I think it's possible to put its link on the
> default
> > >> > > > description
> > >> > > > > > of
> > >> > > > > > > > pull request, so that the user has to see it when
> submits
> > >> the
> > >> > > code.
> > >> > > > > > > >
> > >> > > > > > > > Best, JingsongLee
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > ------------------------------------------------------------------
> > >> > > > > > > > From:Piotr Nowojski <[hidden email]>
> > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03
> > >> > > > > > > > To:dev <[hidden email]>
> > >> > > > > > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality
> > >> Guide
> > >> > > > > > > >
> > >> > > > > > > > +1 for it and general content and thank everybody that
> was
> > >> > > involved
> > >> > > > > in
> > >> > > > > > > creating & writing this down.
> > >> > > > > > > >
> > >> > > > > > > > +1 for splitting it up into some easily navigable
> topics.
> > >> > > > > > > >
> > >> > > > > > > > Piotrek
> > >> > > > > > > >
> > >> > > > > > > >> On 13 Jun 2019, at 09:54, Stephan Ewen <
> [hidden email]
> > >
> > >> > > wrote:
> > >> > > > > > > >>
> > >> > > > > > > >> This should definitely be split up into topics, agreed.
> > >> > > > > > > >> And it should be linked form the "how to contribute"
> page
> > >> or
> > >> > the
> > >> > > > PR
> > >> > > > > > > >> template to make contributors aware.
> > >> > > > > > > >>
> > >> > > > > > > >> On Thu, Jun 13, 2019 at 9:51 AM Zili Chen <
> > >> > [hidden email]
> > >> > > >
> > >> > > > > > wrote:
> > >> > > > > > > >>
> > >> > > > > > > >>> Thanks for creating this guide Stephan. It is also
> > >> > > > > > > >>> a good start point to internals doc.
> > >> > > > > > > >>>
> > >> > > > > > > >>> One suggestion is we could finally separate the guide
> > >> > > > > > > >>> into separated files each of which focus on a specific
> > >> > > > > > > >>> topic. Besides, add the guide to our repository should
> > >> > > > > > > >>> make contributors more aware of it.
> > >> > > > > > > >>>
> > >> > > > > > > >>> Best,
> > >> > > > > > > >>> tison.
> > >> > > > > > > >>>
> > >> > > > > > > >>>
> > >> > > > > > > >>> Jeff Zhang <[hidden email]> 于2019年6月13日周四 下午3:35写道:
> > >> > > > > > > >>>
> > >> > > > > > > >>>> Thanks for the proposal, Stephan. Big +1 on this.
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> This is definitely helpful and indispensable for
> > flink's
> > >> > code
> > >> > > > > > quality
> > >> > > > > > > and
> > >> > > > > > > >>>> healthy community growth.
> > >> > > > > > > >>>> It would also benefit downstream project to integrate
> > >> flink
> > >> > > > > easier.
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> Till Rohrmann <[hidden email]> 于2019年6月13日周四
> > >> > 下午3:29写道:
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>> Thanks for creating this code style and quality
> guide
> > >> > > Stephan.
> > >> > > > I
> > >> > > > > > > think
> > >> > > > > > > >>>>> Flink can benefit from such a guide. Thus +1 for
> > >> > introducing
> > >> > > > and
> > >> > > > > > > >>> adhering
> > >> > > > > > > >>>>> to it.
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>> Cheers,
> > >> > > > > > > >>>>> Till
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>> On Thu, Jun 13, 2019 at 5:26 AM Bowen Li <
> > >> > > [hidden email]>
> > >> > > > > > > wrote:
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>>> Hi Stephan,
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Definitely a good guide in principle IMO! I
> > personally
> > >> > > already
> > >> > > > > > find
> > >> > > > > > > >>> it
> > >> > > > > > > >>>>>> useful in practice to learn from it myself, and
> also
> > >> > promote
> > >> > > > and
> > >> > > > > > > >>>>> cultivate
> > >> > > > > > > >>>>>> a better coding habit around by referencing it. So
> > big
> > >> +1
> > >> > to
> > >> > > > > adopt
> > >> > > > > > > >>> it.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Also want to highlight that we need to make real
> use
> > of
> > >> > it,
> > >> > > > and
> > >> > > > > > keep
> > >> > > > > > > >>>>>> ensuring people are aware of its existence. Adding
> it
> > >> to
> > >> > > Flink
> > >> > > > > > > >>> website
> > >> > > > > > > >>>>>> would be nice. We can also adding noticeable link
> for
> > >> it
> > >> > to
> > >> > > > the
> > >> > > > > > > >>>> template
> > >> > > > > > > >>>>> of
> > >> > > > > > > >>>>>> github PR (where this guide really matters) to get
> > >> > attention
> > >> > > > and
> > >> > > > > > > >>>>> exposure.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> BTW, what's the plan on how people can raise new
> > >> > > > > > > coding-style/quality
> > >> > > > > > > >>>>>> related questions, how to expand and adjust the
> > content
> > >> > over
> > >> > > > > time,
> > >> > > > > > > >>> how
> > >> > > > > > > >>>> to
> > >> > > > > > > >>>>>> inform the community of such updates and changes?
> > >> Maybe we
> > >> > > can
> > >> > > > > use
> > >> > > > > > > >>> some
> > >> > > > > > > >>>>>> tags like "[CODING STYLE]" in dev mailing list for
> > >> these
> > >> > > type
> > >> > > > of
> > >> > > > > > > >>>>>> communications? This can be a separate discussion
> > >> though
> > >> > if
> > >> > > we
> > >> > > > > > wish.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> All in all, big +1 for adopting it.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Bowen
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> On Wed, Jun 12, 2019 at 12:32 PM Stephan Ewen <
> > >> > > > [hidden email]
> > >> > > > > >
> > >> > > > > > > >>>> wrote:
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>> Hi all!
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> I want to propose that we try and adopt a code
> style
> > >> and
> > >> > > > > quality
> > >> > > > > > > >>>> guide.
> > >> > > > > > > >>>>>> Its
> > >> > > > > > > >>>>>>> a big endeavor, but I think it's worth trying :-)
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> The Apache Flink community and contributor base is
> > >> > growing
> > >> > > > > quite
> > >> > > > > > a
> > >> > > > > > > >>>> bit,
> > >> > > > > > > >>>>>> new
> > >> > > > > > > >>>>>>> committers and contributors are coming on board
> > >> > frequently.
> > >> > > > > > > >>> Everyone
> > >> > > > > > > >>>>>> comes
> > >> > > > > > > >>>>>>> from a different background and brings a different
> > >> level
> > >> > of
> > >> > > > > > > >>>> experience.
> > >> > > > > > > >>>>>>> Different contributors apply different styles, and
> > >> > > > > contributions
> > >> > > > > > > >>> are
> > >> > > > > > > >>>>>>> naturally of varying code quality.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> We are struggling with finding a good balance
> > between:
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> (1) On the one hand maintaining a certain code
> > >> standard
> > >> > > for a
> > >> > > > > big
> > >> > > > > > > >>>> and
> > >> > > > > > > >>>>>>> complex system like Flink, to reduce bugs and make
> > >> future
> > >> > > > > > > >>>> contributions
> > >> > > > > > > >>>>>>> feasible (and not impossible due to code
> > complexity).
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> (2) On the other hand, make contributions possible
> > for
> > >> > > > > > > >>>> contributors.
> > >> > > > > > > >>>>>> This
> > >> > > > > > > >>>>>>> means not guarding everything to the point that no
> > >> > > > > contributions
> > >> > > > > > > >>> can
> > >> > > > > > > >>>>> get
> > >> > > > > > > >>>>>> in
> > >> > > > > > > >>>>>>> any more.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> My suggestion to help with this is to define a
> > >> standard
> > >> > and
> > >> > > > > > > >>> document
> > >> > > > > > > >>>> it
> > >> > > > > > > >>>>>>> explicitly. It will help to get everyone on the
> same
> > >> page
> > >> > > > what
> > >> > > > > > the
> > >> > > > > > > >>>>>>> expectation is, and it helps contributors know
> what
> > to
> > >> > pay
> > >> > > > > > > >>> attention
> > >> > > > > > > >>>>> to.
> > >> > > > > > > >>>>>>> Such a guide should also help make review more
> > >> efficient,
> > >> > > > > because
> > >> > > > > > > >>>>>>> contributors can know up front what reviewers will
> > >> look
> > >> > at.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> Over the past weeks, I took a look at the current
> > >> Flink
> > >> > > code
> > >> > > > > base
> > >> > > > > > > >>> and
> > >> > > > > > > >>>>>>> talked to some committers and contributors and
> tried
> > >> to
> > >> > > come
> > >> > > > up
> > >> > > > > > > >>> with
> > >> > > > > > > >>>> a
> > >> > > > > > > >>>>>>> proposal that reflects the approaches and
> standards
> > >> that
> > >> > > many
> > >> > > > > > > >>>>> committers
> > >> > > > > > > >>>>>>> have adopted lately.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> This guide is not fix and final, we should discuss
> > it
> > >> and
> > >> > > > > expand
> > >> > > > > > > >>> and
> > >> > > > > > > >>>>>> adjust
> > >> > > > > > > >>>>>>> it over time. The guide tries to strike a balance
> > >> between
> > >> > > too
> > >> > > > > > much
> > >> > > > > > > >>>>>> contents
> > >> > > > > > > >>>>>>> (don't force someone to read a book before
> > >> contributing)
> > >> > > and
> > >> > > > > > being
> > >> > > > > > > >>>>>>> comprehensive enough. The guide looks long, but
> much
> > >> of
> > >> > it
> > >> > > > are
> > >> > > > > > > >>>>> component
> > >> > > > > > > >>>>>> or
> > >> > > > > > > >>>>>>> aspect specific parts, like how to deal with new
> > >> > > > dependencies,
> > >> > > > > > > >>>>>>> configuration changes, etc.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> I an curious to hear whether you think such a
> guide
> > >> is in
> > >> > > > > > > >>> principle a
> > >> > > > > > > >>>>>> good
> > >> > > > > > > >>>>>>> idea.
> > >> > > > > > > >>>>>>> If yes, is the one here a good starting point?
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> Best,
> > >> > > > > > > >>>>>>> Stephan
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> --
> > >> > > > > > > >>>> Best Regards
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> Jeff Zhang
> > >> > > > > > > >>>>
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Adopting a Code Style and Quality Guide

Robert Metzger
I have received one review for the PR so far.
I'll merge it in the next 24 hours unless there's further feedback.

On Mon, Jul 8, 2019 at 6:04 PM Robert Metzger <[hidden email]> wrote:

> I've converted the google document into a markdown-based pull request
> here: https://github.com/apache/flink-web/pull/224
>
> I didn't put a ton of effort into splitting the guide into multiple pages.
> It would be nice to reach consensus on the exact split in the PR.
>
> On Thu, Jun 27, 2019 at 10:48 PM Hugo Louro <[hidden email]> wrote:
>
>> +1. Thanks for working on the guide. It's very thorough and a good
>> resource
>> to learn good practices from.
>>
>> I would like use this thread as a placeholder for a couple of topics that
>> may be deserving of further discussion on different threads:
>>   - What's the best way to keep track of checkstyle version updates. For
>> instance, currently there is a PR
>> <https://github.com/apache/flink/pull/8870> proposing checkstyle to be
>> updated because version 8.12 is no longer supported
>>  - When classes import shaded dependencies, it is not trivial for IntelliJ
>> to download and associate sources and javadocs, which makes debugging and
>> navigate the code base harder. I tried installing the version of the
>> library using maven from the CLI, and associate the sources "manually" on
>> IntelliJ, but it seems it does not work (perhaps IntelliJ issue). Does
>> anyone know of a good solution? If so, we should added here
>> <
>> https://ci.apache.org/projects/flink/flink-docs-release-1.8/flinkDev/ide_setup.html#intellij-idea
>> >.
>> I can volunteer for that if you tell me how to do it :)
>> - did the community evaluate naming test methods according to these
>> conventions <https://stackoverflow.com/a/1594049> ?
>>
>> Thanks
>>
>> On Mon, Jun 24, 2019 at 11:38 AM Stephan Ewen <[hidden email]> wrote:
>>
>> > I think it makes sense to also start individual [DISCUSS] threads about
>> > extensions and follow-ups.
>> > Various suggestions came up, partly as comments in the doc, some as
>> > questions in other threads.
>> >
>> > Examples:
>> >   - use of null in return types versus Optional versus
>> @Nullable/@Nonnull
>> >   - initialization of collection sizes
>> >   - logging
>> >
>> > I think these would be best discussed in separate threads each.
>> > So, for contributors to whom these issues are dear, feel free to spawn
>> > these additional threads.
>> > (Bear in mind it is close to 1.9 feature freeze time, so please leave
>> this
>> > discussions a bit of time so that all community members have a chance to
>> > participate)
>> >
>> >
>> >
>> > On Mon, Jun 24, 2019 at 7:51 PM Stephan Ewen <[hidden email]> wrote:
>> >
>> > > Thanks for the pointer David.
>> > >
>> > > I was not aware of this tool and I have no experience with its
>> practical
>> > > impact. For example I would be curious what the effect of this is for
>> > > existing PRs, merge conflicts, etc.
>> > >
>> > > If you want, feel free to start another discuss thread on the
>> > introduction
>> > > of such a tool.
>> > >
>> > > On Sun, Jun 23, 2019 at 6:32 PM David Morávek <[hidden email]>
>> wrote:
>> > >
>> > >> I love this kind of unification being pushed forward, the benefit it
>> has
>> > >> on
>> > >> code reviews is enormous. Just my two cents, did you guys think about
>> > >> adopting an automatic code formatter for java, the same way as Apache
>> > Beam
>> > >> did?
>> > >>
>> > >> It is super easy to use for contributors as they don't need to keep
>> any
>> > >> particular coding style in mind and they can only focus on
>> functionality
>> > >> they want to fix, and it's also great for reviewers, because they
>> only
>> > see
>> > >> the important changes. This also eliminates need for any special
>> editor
>> > /
>> > >> checkstyle configs as the code formatting is part of the build
>> itself.
>> > >>
>> > >> The one Beam uses is https://github.com/diffplug/spotless with
>> > >> GoogleJavaFormat, it may be worth to look into.
>> > >>
>> > >> Best,
>> > >> David
>> > >>
>> > >> On Fri, Jun 21, 2019 at 4:40 PM Stephan Ewen <[hidden email]>
>> wrote:
>> > >>
>> > >> > Thanks, everyone, for the positive feedback :-)
>> > >> >
>> > >> > @Robert - It probably makes sense to break this down into various
>> > pages,
>> > >> > like PR, general code style guide, Java, component specific guides,
>> > >> > formats, etc.
>> > >> >
>> > >> > Best,
>> > >> > Stephan
>> > >> >
>> > >> >
>> > >> > On Fri, Jun 21, 2019 at 4:29 PM Robert Metzger <
>> [hidden email]>
>> > >> > wrote:
>> > >> >
>> > >> > > It seems that the discussion around this topic has settled.
>> > >> > >
>> > >> > > I'm going to turn the Google Doc into a markdown file (maybe also
>> > >> > multiple,
>> > >> > > I'll try out different things) and then open a pull request for
>> the
>> > >> Flink
>> > >> > > website.
>> > >> > > I'll post a link to the PR here once I'm done.
>> > >> > >
>> > >> > > On Fri, Jun 14, 2019 at 9:36 AM zhijiang <
>> > [hidden email]
>> > >> > > .invalid>
>> > >> > > wrote:
>> > >> > >
>> > >> > > > Thanks for providing this useful guide for benefiting both
>> > >> contributors
>> > >> > > > and committers in consistency.
>> > >> > > >
>> > >> > > > I just reviewed and learned the whole doc which covers a lot of
>> > >> > > > information. Wish it further categoried and put onto somewhere
>> for
>> > >> > easily
>> > >> > > > traced future.
>> > >> > > >
>> > >> > > > Best,
>> > >> > > > Zhijiang
>> > >> > > >
>> ------------------------------------------------------------------
>> > >> > > > From:Robert Metzger <[hidden email]>
>> > >> > > > Send Time:2019年6月14日(星期五) 14:24
>> > >> > > > To:dev <[hidden email]>
>> > >> > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality Guide
>> > >> > > >
>> > >> > > > Thanks a lot for putting this together!
>> > >> > > >
>> > >> > > > I'm in the process of reworking the "How to contribute" pages
>> [1]
>> > >> and
>> > >> > I'm
>> > >> > > > happy to add the guide to the Flink website, once the
>> discussion
>> > >> here
>> > >> > is
>> > >> > > > over.
>> > >> > > >
>> > >> > > > [1] https://github.com/apache/flink-web/pull/217
>> > >> > > >
>> > >> > > > On Fri, Jun 14, 2019 at 3:21 AM Kurt Young <[hidden email]>
>> > >> wrote:
>> > >> > > >
>> > >> > > > > Big +1 and thanks for preparing this.
>> > >> > > > >
>> > >> > > > > I think wha't more important is making sure most all the
>> > >> contributors
>> > >> > > can
>> > >> > > > > follow
>> > >> > > > > the same guide, a clear document is definitely a great start.
>> > >> > > Committers
>> > >> > > > > can
>> > >> > > > > first try to follow the guide by self, and spread the
>> standard
>> > >> during
>> > >> > > > code
>> > >> > > > > reviewing.
>> > >> > > > >
>> > >> > > > > Best,
>> > >> > > > > Kurt
>> > >> > > > >
>> > >> > > > >
>> > >> > > > > On Thu, Jun 13, 2019 at 8:28 PM Congxian Qiu <
>> > >> [hidden email]
>> > >> > >
>> > >> > > > > wrote:
>> > >> > > > >
>> > >> > > > > > +1 for this, I think all contributors can benefit from
>> this.
>> > >> > > > > >
>> > >> > > > > > Best,
>> > >> > > > > > Congxian
>> > >> > > > > >
>> > >> > > > > >
>> > >> > > > > > Aljoscha Krettek <[hidden email]> 于2019年6月13日周四
>> > 下午8:14写道:
>> > >> > > > > >
>> > >> > > > > > > +1 I think this is a very good effort and should put to
>> rest
>> > >> some
>> > >> > > > > > > back-and-forth discussions on PRs and some differences in
>> > >> “style”
>> > >> > > > > between
>> > >> > > > > > > committers. ;-)
>> > >> > > > > > >
>> > >> > > > > > > > On 13. Jun 2019, at 10:21, JingsongLee <
>> > >> > [hidden email]
>> > >> > > > > > .INVALID>
>> > >> > > > > > > wrote:
>> > >> > > > > > > >
>> > >> > > > > > > > big +1, the content is very useful and enlightening.
>> > >> > > > > > > > But it's really too long to look at.
>> > >> > > > > > > > +1 for splitting it and expose it to contributors.
>> > >> > > > > > > >
>> > >> > > > > > > > Even I think it's possible to put its link on the
>> default
>> > >> > > > description
>> > >> > > > > > of
>> > >> > > > > > > > pull request, so that the user has to see it when
>> submits
>> > >> the
>> > >> > > code.
>> > >> > > > > > > >
>> > >> > > > > > > > Best, JingsongLee
>> > >> > > > > > > >
>> > >> > > > > > > >
>> > >> > > > > > > >
>> > >> > >
>> ------------------------------------------------------------------
>> > >> > > > > > > > From:Piotr Nowojski <[hidden email]>
>> > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03
>> > >> > > > > > > > To:dev <[hidden email]>
>> > >> > > > > > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality
>> > >> Guide
>> > >> > > > > > > >
>> > >> > > > > > > > +1 for it and general content and thank everybody that
>> was
>> > >> > > involved
>> > >> > > > > in
>> > >> > > > > > > creating & writing this down.
>> > >> > > > > > > >
>> > >> > > > > > > > +1 for splitting it up into some easily navigable
>> topics.
>> > >> > > > > > > >
>> > >> > > > > > > > Piotrek
>> > >> > > > > > > >
>> > >> > > > > > > >> On 13 Jun 2019, at 09:54, Stephan Ewen <
>> [hidden email]
>> > >
>> > >> > > wrote:
>> > >> > > > > > > >>
>> > >> > > > > > > >> This should definitely be split up into topics,
>> agreed.
>> > >> > > > > > > >> And it should be linked form the "how to contribute"
>> page
>> > >> or
>> > >> > the
>> > >> > > > PR
>> > >> > > > > > > >> template to make contributors aware.
>> > >> > > > > > > >>
>> > >> > > > > > > >> On Thu, Jun 13, 2019 at 9:51 AM Zili Chen <
>> > >> > [hidden email]
>> > >> > > >
>> > >> > > > > > wrote:
>> > >> > > > > > > >>
>> > >> > > > > > > >>> Thanks for creating this guide Stephan. It is also
>> > >> > > > > > > >>> a good start point to internals doc.
>> > >> > > > > > > >>>
>> > >> > > > > > > >>> One suggestion is we could finally separate the guide
>> > >> > > > > > > >>> into separated files each of which focus on a
>> specific
>> > >> > > > > > > >>> topic. Besides, add the guide to our repository
>> should
>> > >> > > > > > > >>> make contributors more aware of it.
>> > >> > > > > > > >>>
>> > >> > > > > > > >>> Best,
>> > >> > > > > > > >>> tison.
>> > >> > > > > > > >>>
>> > >> > > > > > > >>>
>> > >> > > > > > > >>> Jeff Zhang <[hidden email]> 于2019年6月13日周四
>> 下午3:35写道:
>> > >> > > > > > > >>>
>> > >> > > > > > > >>>> Thanks for the proposal, Stephan. Big +1 on this.
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>> This is definitely helpful and indispensable for
>> > flink's
>> > >> > code
>> > >> > > > > > quality
>> > >> > > > > > > and
>> > >> > > > > > > >>>> healthy community growth.
>> > >> > > > > > > >>>> It would also benefit downstream project to
>> integrate
>> > >> flink
>> > >> > > > > easier.
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>> Till Rohrmann <[hidden email]> 于2019年6月13日周四
>> > >> > 下午3:29写道:
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>>> Thanks for creating this code style and quality
>> guide
>> > >> > > Stephan.
>> > >> > > > I
>> > >> > > > > > > think
>> > >> > > > > > > >>>>> Flink can benefit from such a guide. Thus +1 for
>> > >> > introducing
>> > >> > > > and
>> > >> > > > > > > >>> adhering
>> > >> > > > > > > >>>>> to it.
>> > >> > > > > > > >>>>>
>> > >> > > > > > > >>>>> Cheers,
>> > >> > > > > > > >>>>> Till
>> > >> > > > > > > >>>>>
>> > >> > > > > > > >>>>> On Thu, Jun 13, 2019 at 5:26 AM Bowen Li <
>> > >> > > [hidden email]>
>> > >> > > > > > > wrote:
>> > >> > > > > > > >>>>>
>> > >> > > > > > > >>>>>> Hi Stephan,
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> Definitely a good guide in principle IMO! I
>> > personally
>> > >> > > already
>> > >> > > > > > find
>> > >> > > > > > > >>> it
>> > >> > > > > > > >>>>>> useful in practice to learn from it myself, and
>> also
>> > >> > promote
>> > >> > > > and
>> > >> > > > > > > >>>>> cultivate
>> > >> > > > > > > >>>>>> a better coding habit around by referencing it. So
>> > big
>> > >> +1
>> > >> > to
>> > >> > > > > adopt
>> > >> > > > > > > >>> it.
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> Also want to highlight that we need to make real
>> use
>> > of
>> > >> > it,
>> > >> > > > and
>> > >> > > > > > keep
>> > >> > > > > > > >>>>>> ensuring people are aware of its existence.
>> Adding it
>> > >> to
>> > >> > > Flink
>> > >> > > > > > > >>> website
>> > >> > > > > > > >>>>>> would be nice. We can also adding noticeable link
>> for
>> > >> it
>> > >> > to
>> > >> > > > the
>> > >> > > > > > > >>>> template
>> > >> > > > > > > >>>>> of
>> > >> > > > > > > >>>>>> github PR (where this guide really matters) to get
>> > >> > attention
>> > >> > > > and
>> > >> > > > > > > >>>>> exposure.
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> BTW, what's the plan on how people can raise new
>> > >> > > > > > > coding-style/quality
>> > >> > > > > > > >>>>>> related questions, how to expand and adjust the
>> > content
>> > >> > over
>> > >> > > > > time,
>> > >> > > > > > > >>> how
>> > >> > > > > > > >>>> to
>> > >> > > > > > > >>>>>> inform the community of such updates and changes?
>> > >> Maybe we
>> > >> > > can
>> > >> > > > > use
>> > >> > > > > > > >>> some
>> > >> > > > > > > >>>>>> tags like "[CODING STYLE]" in dev mailing list for
>> > >> these
>> > >> > > type
>> > >> > > > of
>> > >> > > > > > > >>>>>> communications? This can be a separate discussion
>> > >> though
>> > >> > if
>> > >> > > we
>> > >> > > > > > wish.
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> All in all, big +1 for adopting it.
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> Bowen
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>> On Wed, Jun 12, 2019 at 12:32 PM Stephan Ewen <
>> > >> > > > [hidden email]
>> > >> > > > > >
>> > >> > > > > > > >>>> wrote:
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>>> Hi all!
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> I want to propose that we try and adopt a code
>> style
>> > >> and
>> > >> > > > > quality
>> > >> > > > > > > >>>> guide.
>> > >> > > > > > > >>>>>> Its
>> > >> > > > > > > >>>>>>> a big endeavor, but I think it's worth trying :-)
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> The Apache Flink community and contributor base
>> is
>> > >> > growing
>> > >> > > > > quite
>> > >> > > > > > a
>> > >> > > > > > > >>>> bit,
>> > >> > > > > > > >>>>>> new
>> > >> > > > > > > >>>>>>> committers and contributors are coming on board
>> > >> > frequently.
>> > >> > > > > > > >>> Everyone
>> > >> > > > > > > >>>>>> comes
>> > >> > > > > > > >>>>>>> from a different background and brings a
>> different
>> > >> level
>> > >> > of
>> > >> > > > > > > >>>> experience.
>> > >> > > > > > > >>>>>>> Different contributors apply different styles,
>> and
>> > >> > > > > contributions
>> > >> > > > > > > >>> are
>> > >> > > > > > > >>>>>>> naturally of varying code quality.
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> We are struggling with finding a good balance
>> > between:
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> (1) On the one hand maintaining a certain code
>> > >> standard
>> > >> > > for a
>> > >> > > > > big
>> > >> > > > > > > >>>> and
>> > >> > > > > > > >>>>>>> complex system like Flink, to reduce bugs and
>> make
>> > >> future
>> > >> > > > > > > >>>> contributions
>> > >> > > > > > > >>>>>>> feasible (and not impossible due to code
>> > complexity).
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> (2) On the other hand, make contributions
>> possible
>> > for
>> > >> > > > > > > >>>> contributors.
>> > >> > > > > > > >>>>>> This
>> > >> > > > > > > >>>>>>> means not guarding everything to the point that
>> no
>> > >> > > > > contributions
>> > >> > > > > > > >>> can
>> > >> > > > > > > >>>>> get
>> > >> > > > > > > >>>>>> in
>> > >> > > > > > > >>>>>>> any more.
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> My suggestion to help with this is to define a
>> > >> standard
>> > >> > and
>> > >> > > > > > > >>> document
>> > >> > > > > > > >>>> it
>> > >> > > > > > > >>>>>>> explicitly. It will help to get everyone on the
>> same
>> > >> page
>> > >> > > > what
>> > >> > > > > > the
>> > >> > > > > > > >>>>>>> expectation is, and it helps contributors know
>> what
>> > to
>> > >> > pay
>> > >> > > > > > > >>> attention
>> > >> > > > > > > >>>>> to.
>> > >> > > > > > > >>>>>>> Such a guide should also help make review more
>> > >> efficient,
>> > >> > > > > because
>> > >> > > > > > > >>>>>>> contributors can know up front what reviewers
>> will
>> > >> look
>> > >> > at.
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> Over the past weeks, I took a look at the current
>> > >> Flink
>> > >> > > code
>> > >> > > > > base
>> > >> > > > > > > >>> and
>> > >> > > > > > > >>>>>>> talked to some committers and contributors and
>> tried
>> > >> to
>> > >> > > come
>> > >> > > > up
>> > >> > > > > > > >>> with
>> > >> > > > > > > >>>> a
>> > >> > > > > > > >>>>>>> proposal that reflects the approaches and
>> standards
>> > >> that
>> > >> > > many
>> > >> > > > > > > >>>>> committers
>> > >> > > > > > > >>>>>>> have adopted lately.
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> >
>> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> This guide is not fix and final, we should
>> discuss
>> > it
>> > >> and
>> > >> > > > > expand
>> > >> > > > > > > >>> and
>> > >> > > > > > > >>>>>> adjust
>> > >> > > > > > > >>>>>>> it over time. The guide tries to strike a balance
>> > >> between
>> > >> > > too
>> > >> > > > > > much
>> > >> > > > > > > >>>>>> contents
>> > >> > > > > > > >>>>>>> (don't force someone to read a book before
>> > >> contributing)
>> > >> > > and
>> > >> > > > > > being
>> > >> > > > > > > >>>>>>> comprehensive enough. The guide looks long, but
>> much
>> > >> of
>> > >> > it
>> > >> > > > are
>> > >> > > > > > > >>>>> component
>> > >> > > > > > > >>>>>> or
>> > >> > > > > > > >>>>>>> aspect specific parts, like how to deal with new
>> > >> > > > dependencies,
>> > >> > > > > > > >>>>>>> configuration changes, etc.
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> I an curious to hear whether you think such a
>> guide
>> > >> is in
>> > >> > > > > > > >>> principle a
>> > >> > > > > > > >>>>>> good
>> > >> > > > > > > >>>>>>> idea.
>> > >> > > > > > > >>>>>>> If yes, is the one here a good starting point?
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>> Best,
>> > >> > > > > > > >>>>>>> Stephan
>> > >> > > > > > > >>>>>>>
>> > >> > > > > > > >>>>>>
>> > >> > > > > > > >>>>>
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>> --
>> > >> > > > > > > >>>> Best Regards
>> > >> > > > > > > >>>>
>> > >> > > > > > > >>>> Jeff Zhang
>> > >> > > > > > > >>>>
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> > >
>> >
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Adopting a Code Style and Quality Guide

Stephan Ewen
In reply to this post by Hugo Louro
@hugo For your suggestions, I would ask to start a separate discussion
thread.
I think this mail thread has converged towards merging the initial
suggestion as a starting point and refining it later based on new
discussions.

Best,
Stephan


On Thu, Jun 27, 2019 at 10:48 PM Hugo Louro <[hidden email]> wrote:

> +1. Thanks for working on the guide. It's very thorough and a good resource
> to learn good practices from.
>
> I would like use this thread as a placeholder for a couple of topics that
> may be deserving of further discussion on different threads:
>   - What's the best way to keep track of checkstyle version updates. For
> instance, currently there is a PR
> <https://github.com/apache/flink/pull/8870> proposing checkstyle to be
> updated because version 8.12 is no longer supported
>  - When classes import shaded dependencies, it is not trivial for IntelliJ
> to download and associate sources and javadocs, which makes debugging and
> navigate the code base harder. I tried installing the version of the
> library using maven from the CLI, and associate the sources "manually" on
> IntelliJ, but it seems it does not work (perhaps IntelliJ issue). Does
> anyone know of a good solution? If so, we should added here
> <
> https://ci.apache.org/projects/flink/flink-docs-release-1.8/flinkDev/ide_setup.html#intellij-idea
> >.
> I can volunteer for that if you tell me how to do it :)
> - did the community evaluate naming test methods according to these
> conventions <https://stackoverflow.com/a/1594049> ?
>
> Thanks
>
> On Mon, Jun 24, 2019 at 11:38 AM Stephan Ewen <[hidden email]> wrote:
>
> > I think it makes sense to also start individual [DISCUSS] threads about
> > extensions and follow-ups.
> > Various suggestions came up, partly as comments in the doc, some as
> > questions in other threads.
> >
> > Examples:
> >   - use of null in return types versus Optional versus @Nullable/@Nonnull
> >   - initialization of collection sizes
> >   - logging
> >
> > I think these would be best discussed in separate threads each.
> > So, for contributors to whom these issues are dear, feel free to spawn
> > these additional threads.
> > (Bear in mind it is close to 1.9 feature freeze time, so please leave
> this
> > discussions a bit of time so that all community members have a chance to
> > participate)
> >
> >
> >
> > On Mon, Jun 24, 2019 at 7:51 PM Stephan Ewen <[hidden email]> wrote:
> >
> > > Thanks for the pointer David.
> > >
> > > I was not aware of this tool and I have no experience with its
> practical
> > > impact. For example I would be curious what the effect of this is for
> > > existing PRs, merge conflicts, etc.
> > >
> > > If you want, feel free to start another discuss thread on the
> > introduction
> > > of such a tool.
> > >
> > > On Sun, Jun 23, 2019 at 6:32 PM David Morávek <[hidden email]> wrote:
> > >
> > >> I love this kind of unification being pushed forward, the benefit it
> has
> > >> on
> > >> code reviews is enormous. Just my two cents, did you guys think about
> > >> adopting an automatic code formatter for java, the same way as Apache
> > Beam
> > >> did?
> > >>
> > >> It is super easy to use for contributors as they don't need to keep
> any
> > >> particular coding style in mind and they can only focus on
> functionality
> > >> they want to fix, and it's also great for reviewers, because they only
> > see
> > >> the important changes. This also eliminates need for any special
> editor
> > /
> > >> checkstyle configs as the code formatting is part of the build itself.
> > >>
> > >> The one Beam uses is https://github.com/diffplug/spotless with
> > >> GoogleJavaFormat, it may be worth to look into.
> > >>
> > >> Best,
> > >> David
> > >>
> > >> On Fri, Jun 21, 2019 at 4:40 PM Stephan Ewen <[hidden email]>
> wrote:
> > >>
> > >> > Thanks, everyone, for the positive feedback :-)
> > >> >
> > >> > @Robert - It probably makes sense to break this down into various
> > pages,
> > >> > like PR, general code style guide, Java, component specific guides,
> > >> > formats, etc.
> > >> >
> > >> > Best,
> > >> > Stephan
> > >> >
> > >> >
> > >> > On Fri, Jun 21, 2019 at 4:29 PM Robert Metzger <[hidden email]
> >
> > >> > wrote:
> > >> >
> > >> > > It seems that the discussion around this topic has settled.
> > >> > >
> > >> > > I'm going to turn the Google Doc into a markdown file (maybe also
> > >> > multiple,
> > >> > > I'll try out different things) and then open a pull request for
> the
> > >> Flink
> > >> > > website.
> > >> > > I'll post a link to the PR here once I'm done.
> > >> > >
> > >> > > On Fri, Jun 14, 2019 at 9:36 AM zhijiang <
> > [hidden email]
> > >> > > .invalid>
> > >> > > wrote:
> > >> > >
> > >> > > > Thanks for providing this useful guide for benefiting both
> > >> contributors
> > >> > > > and committers in consistency.
> > >> > > >
> > >> > > > I just reviewed and learned the whole doc which covers a lot of
> > >> > > > information. Wish it further categoried and put onto somewhere
> for
> > >> > easily
> > >> > > > traced future.
> > >> > > >
> > >> > > > Best,
> > >> > > > Zhijiang
> > >> > > >
> ------------------------------------------------------------------
> > >> > > > From:Robert Metzger <[hidden email]>
> > >> > > > Send Time:2019年6月14日(星期五) 14:24
> > >> > > > To:dev <[hidden email]>
> > >> > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality Guide
> > >> > > >
> > >> > > > Thanks a lot for putting this together!
> > >> > > >
> > >> > > > I'm in the process of reworking the "How to contribute" pages
> [1]
> > >> and
> > >> > I'm
> > >> > > > happy to add the guide to the Flink website, once the discussion
> > >> here
> > >> > is
> > >> > > > over.
> > >> > > >
> > >> > > > [1] https://github.com/apache/flink-web/pull/217
> > >> > > >
> > >> > > > On Fri, Jun 14, 2019 at 3:21 AM Kurt Young <[hidden email]>
> > >> wrote:
> > >> > > >
> > >> > > > > Big +1 and thanks for preparing this.
> > >> > > > >
> > >> > > > > I think wha't more important is making sure most all the
> > >> contributors
> > >> > > can
> > >> > > > > follow
> > >> > > > > the same guide, a clear document is definitely a great start.
> > >> > > Committers
> > >> > > > > can
> > >> > > > > first try to follow the guide by self, and spread the standard
> > >> during
> > >> > > > code
> > >> > > > > reviewing.
> > >> > > > >
> > >> > > > > Best,
> > >> > > > > Kurt
> > >> > > > >
> > >> > > > >
> > >> > > > > On Thu, Jun 13, 2019 at 8:28 PM Congxian Qiu <
> > >> [hidden email]
> > >> > >
> > >> > > > > wrote:
> > >> > > > >
> > >> > > > > > +1 for this, I think all contributors can benefit from this.
> > >> > > > > >
> > >> > > > > > Best,
> > >> > > > > > Congxian
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > Aljoscha Krettek <[hidden email]> 于2019年6月13日周四
> > 下午8:14写道:
> > >> > > > > >
> > >> > > > > > > +1 I think this is a very good effort and should put to
> rest
> > >> some
> > >> > > > > > > back-and-forth discussions on PRs and some differences in
> > >> “style”
> > >> > > > > between
> > >> > > > > > > committers. ;-)
> > >> > > > > > >
> > >> > > > > > > > On 13. Jun 2019, at 10:21, JingsongLee <
> > >> > [hidden email]
> > >> > > > > > .INVALID>
> > >> > > > > > > wrote:
> > >> > > > > > > >
> > >> > > > > > > > big +1, the content is very useful and enlightening.
> > >> > > > > > > > But it's really too long to look at.
> > >> > > > > > > > +1 for splitting it and expose it to contributors.
> > >> > > > > > > >
> > >> > > > > > > > Even I think it's possible to put its link on the
> default
> > >> > > > description
> > >> > > > > > of
> > >> > > > > > > > pull request, so that the user has to see it when
> submits
> > >> the
> > >> > > code.
> > >> > > > > > > >
> > >> > > > > > > > Best, JingsongLee
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > ------------------------------------------------------------------
> > >> > > > > > > > From:Piotr Nowojski <[hidden email]>
> > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03
> > >> > > > > > > > To:dev <[hidden email]>
> > >> > > > > > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality
> > >> Guide
> > >> > > > > > > >
> > >> > > > > > > > +1 for it and general content and thank everybody that
> was
> > >> > > involved
> > >> > > > > in
> > >> > > > > > > creating & writing this down.
> > >> > > > > > > >
> > >> > > > > > > > +1 for splitting it up into some easily navigable
> topics.
> > >> > > > > > > >
> > >> > > > > > > > Piotrek
> > >> > > > > > > >
> > >> > > > > > > >> On 13 Jun 2019, at 09:54, Stephan Ewen <
> [hidden email]
> > >
> > >> > > wrote:
> > >> > > > > > > >>
> > >> > > > > > > >> This should definitely be split up into topics, agreed.
> > >> > > > > > > >> And it should be linked form the "how to contribute"
> page
> > >> or
> > >> > the
> > >> > > > PR
> > >> > > > > > > >> template to make contributors aware.
> > >> > > > > > > >>
> > >> > > > > > > >> On Thu, Jun 13, 2019 at 9:51 AM Zili Chen <
> > >> > [hidden email]
> > >> > > >
> > >> > > > > > wrote:
> > >> > > > > > > >>
> > >> > > > > > > >>> Thanks for creating this guide Stephan. It is also
> > >> > > > > > > >>> a good start point to internals doc.
> > >> > > > > > > >>>
> > >> > > > > > > >>> One suggestion is we could finally separate the guide
> > >> > > > > > > >>> into separated files each of which focus on a specific
> > >> > > > > > > >>> topic. Besides, add the guide to our repository should
> > >> > > > > > > >>> make contributors more aware of it.
> > >> > > > > > > >>>
> > >> > > > > > > >>> Best,
> > >> > > > > > > >>> tison.
> > >> > > > > > > >>>
> > >> > > > > > > >>>
> > >> > > > > > > >>> Jeff Zhang <[hidden email]> 于2019年6月13日周四 下午3:35写道:
> > >> > > > > > > >>>
> > >> > > > > > > >>>> Thanks for the proposal, Stephan. Big +1 on this.
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> This is definitely helpful and indispensable for
> > flink's
> > >> > code
> > >> > > > > > quality
> > >> > > > > > > and
> > >> > > > > > > >>>> healthy community growth.
> > >> > > > > > > >>>> It would also benefit downstream project to integrate
> > >> flink
> > >> > > > > easier.
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> Till Rohrmann <[hidden email]> 于2019年6月13日周四
> > >> > 下午3:29写道:
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>> Thanks for creating this code style and quality
> guide
> > >> > > Stephan.
> > >> > > > I
> > >> > > > > > > think
> > >> > > > > > > >>>>> Flink can benefit from such a guide. Thus +1 for
> > >> > introducing
> > >> > > > and
> > >> > > > > > > >>> adhering
> > >> > > > > > > >>>>> to it.
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>> Cheers,
> > >> > > > > > > >>>>> Till
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>> On Thu, Jun 13, 2019 at 5:26 AM Bowen Li <
> > >> > > [hidden email]>
> > >> > > > > > > wrote:
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>>> Hi Stephan,
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Definitely a good guide in principle IMO! I
> > personally
> > >> > > already
> > >> > > > > > find
> > >> > > > > > > >>> it
> > >> > > > > > > >>>>>> useful in practice to learn from it myself, and
> also
> > >> > promote
> > >> > > > and
> > >> > > > > > > >>>>> cultivate
> > >> > > > > > > >>>>>> a better coding habit around by referencing it. So
> > big
> > >> +1
> > >> > to
> > >> > > > > adopt
> > >> > > > > > > >>> it.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Also want to highlight that we need to make real
> use
> > of
> > >> > it,
> > >> > > > and
> > >> > > > > > keep
> > >> > > > > > > >>>>>> ensuring people are aware of its existence. Adding
> it
> > >> to
> > >> > > Flink
> > >> > > > > > > >>> website
> > >> > > > > > > >>>>>> would be nice. We can also adding noticeable link
> for
> > >> it
> > >> > to
> > >> > > > the
> > >> > > > > > > >>>> template
> > >> > > > > > > >>>>> of
> > >> > > > > > > >>>>>> github PR (where this guide really matters) to get
> > >> > attention
> > >> > > > and
> > >> > > > > > > >>>>> exposure.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> BTW, what's the plan on how people can raise new
> > >> > > > > > > coding-style/quality
> > >> > > > > > > >>>>>> related questions, how to expand and adjust the
> > content
> > >> > over
> > >> > > > > time,
> > >> > > > > > > >>> how
> > >> > > > > > > >>>> to
> > >> > > > > > > >>>>>> inform the community of such updates and changes?
> > >> Maybe we
> > >> > > can
> > >> > > > > use
> > >> > > > > > > >>> some
> > >> > > > > > > >>>>>> tags like "[CODING STYLE]" in dev mailing list for
> > >> these
> > >> > > type
> > >> > > > of
> > >> > > > > > > >>>>>> communications? This can be a separate discussion
> > >> though
> > >> > if
> > >> > > we
> > >> > > > > > wish.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> All in all, big +1 for adopting it.
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> Bowen
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>> On Wed, Jun 12, 2019 at 12:32 PM Stephan Ewen <
> > >> > > > [hidden email]
> > >> > > > > >
> > >> > > > > > > >>>> wrote:
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>>> Hi all!
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> I want to propose that we try and adopt a code
> style
> > >> and
> > >> > > > > quality
> > >> > > > > > > >>>> guide.
> > >> > > > > > > >>>>>> Its
> > >> > > > > > > >>>>>>> a big endeavor, but I think it's worth trying :-)
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> The Apache Flink community and contributor base is
> > >> > growing
> > >> > > > > quite
> > >> > > > > > a
> > >> > > > > > > >>>> bit,
> > >> > > > > > > >>>>>> new
> > >> > > > > > > >>>>>>> committers and contributors are coming on board
> > >> > frequently.
> > >> > > > > > > >>> Everyone
> > >> > > > > > > >>>>>> comes
> > >> > > > > > > >>>>>>> from a different background and brings a different
> > >> level
> > >> > of
> > >> > > > > > > >>>> experience.
> > >> > > > > > > >>>>>>> Different contributors apply different styles, and
> > >> > > > > contributions
> > >> > > > > > > >>> are
> > >> > > > > > > >>>>>>> naturally of varying code quality.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> We are struggling with finding a good balance
> > between:
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> (1) On the one hand maintaining a certain code
> > >> standard
> > >> > > for a
> > >> > > > > big
> > >> > > > > > > >>>> and
> > >> > > > > > > >>>>>>> complex system like Flink, to reduce bugs and make
> > >> future
> > >> > > > > > > >>>> contributions
> > >> > > > > > > >>>>>>> feasible (and not impossible due to code
> > complexity).
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> (2) On the other hand, make contributions possible
> > for
> > >> > > > > > > >>>> contributors.
> > >> > > > > > > >>>>>> This
> > >> > > > > > > >>>>>>> means not guarding everything to the point that no
> > >> > > > > contributions
> > >> > > > > > > >>> can
> > >> > > > > > > >>>>> get
> > >> > > > > > > >>>>>> in
> > >> > > > > > > >>>>>>> any more.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> My suggestion to help with this is to define a
> > >> standard
> > >> > and
> > >> > > > > > > >>> document
> > >> > > > > > > >>>> it
> > >> > > > > > > >>>>>>> explicitly. It will help to get everyone on the
> same
> > >> page
> > >> > > > what
> > >> > > > > > the
> > >> > > > > > > >>>>>>> expectation is, and it helps contributors know
> what
> > to
> > >> > pay
> > >> > > > > > > >>> attention
> > >> > > > > > > >>>>> to.
> > >> > > > > > > >>>>>>> Such a guide should also help make review more
> > >> efficient,
> > >> > > > > because
> > >> > > > > > > >>>>>>> contributors can know up front what reviewers will
> > >> look
> > >> > at.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> Over the past weeks, I took a look at the current
> > >> Flink
> > >> > > code
> > >> > > > > base
> > >> > > > > > > >>> and
> > >> > > > > > > >>>>>>> talked to some committers and contributors and
> tried
> > >> to
> > >> > > come
> > >> > > > up
> > >> > > > > > > >>> with
> > >> > > > > > > >>>> a
> > >> > > > > > > >>>>>>> proposal that reflects the approaches and
> standards
> > >> that
> > >> > > many
> > >> > > > > > > >>>>> committers
> > >> > > > > > > >>>>>>> have adopted lately.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> This guide is not fix and final, we should discuss
> > it
> > >> and
> > >> > > > > expand
> > >> > > > > > > >>> and
> > >> > > > > > > >>>>>> adjust
> > >> > > > > > > >>>>>>> it over time. The guide tries to strike a balance
> > >> between
> > >> > > too
> > >> > > > > > much
> > >> > > > > > > >>>>>> contents
> > >> > > > > > > >>>>>>> (don't force someone to read a book before
> > >> contributing)
> > >> > > and
> > >> > > > > > being
> > >> > > > > > > >>>>>>> comprehensive enough. The guide looks long, but
> much
> > >> of
> > >> > it
> > >> > > > are
> > >> > > > > > > >>>>> component
> > >> > > > > > > >>>>>> or
> > >> > > > > > > >>>>>>> aspect specific parts, like how to deal with new
> > >> > > > dependencies,
> > >> > > > > > > >>>>>>> configuration changes, etc.
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> I an curious to hear whether you think such a
> guide
> > >> is in
> > >> > > > > > > >>> principle a
> > >> > > > > > > >>>>>> good
> > >> > > > > > > >>>>>>> idea.
> > >> > > > > > > >>>>>>> If yes, is the one here a good starting point?
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>> Best,
> > >> > > > > > > >>>>>>> Stephan
> > >> > > > > > > >>>>>>>
> > >> > > > > > > >>>>>>
> > >> > > > > > > >>>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> --
> > >> > > > > > > >>>> Best Regards
> > >> > > > > > > >>>>
> > >> > > > > > > >>>> Jeff Zhang
> > >> > > > > > > >>>>
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Adopting a Code Style and Quality Guide

Hugo Louro
Sounds good @stephan! Thanks.

On Tue, Jul 23, 2019 at 10:15 AM Stephan Ewen <[hidden email]> wrote:

> @hugo For your suggestions, I would ask to start a separate discussion
> thread.
> I think this mail thread has converged towards merging the initial
> suggestion as a starting point and refining it later based on new
> discussions.
>
> Best,
> Stephan
>
>
> On Thu, Jun 27, 2019 at 10:48 PM Hugo Louro <[hidden email]> wrote:
>
> > +1. Thanks for working on the guide. It's very thorough and a good
> resource
> > to learn good practices from.
> >
> > I would like use this thread as a placeholder for a couple of topics that
> > may be deserving of further discussion on different threads:
> >   - What's the best way to keep track of checkstyle version updates. For
> > instance, currently there is a PR
> > <https://github.com/apache/flink/pull/8870> proposing checkstyle to be
> > updated because version 8.12 is no longer supported
> >  - When classes import shaded dependencies, it is not trivial for
> IntelliJ
> > to download and associate sources and javadocs, which makes debugging and
> > navigate the code base harder. I tried installing the version of the
> > library using maven from the CLI, and associate the sources "manually" on
> > IntelliJ, but it seems it does not work (perhaps IntelliJ issue). Does
> > anyone know of a good solution? If so, we should added here
> > <
> >
> https://ci.apache.org/projects/flink/flink-docs-release-1.8/flinkDev/ide_setup.html#intellij-idea
> > >.
> > I can volunteer for that if you tell me how to do it :)
> > - did the community evaluate naming test methods according to these
> > conventions <https://stackoverflow.com/a/1594049> ?
> >
> > Thanks
> >
> > On Mon, Jun 24, 2019 at 11:38 AM Stephan Ewen <[hidden email]> wrote:
> >
> > > I think it makes sense to also start individual [DISCUSS] threads about
> > > extensions and follow-ups.
> > > Various suggestions came up, partly as comments in the doc, some as
> > > questions in other threads.
> > >
> > > Examples:
> > >   - use of null in return types versus Optional versus
> @Nullable/@Nonnull
> > >   - initialization of collection sizes
> > >   - logging
> > >
> > > I think these would be best discussed in separate threads each.
> > > So, for contributors to whom these issues are dear, feel free to spawn
> > > these additional threads.
> > > (Bear in mind it is close to 1.9 feature freeze time, so please leave
> > this
> > > discussions a bit of time so that all community members have a chance
> to
> > > participate)
> > >
> > >
> > >
> > > On Mon, Jun 24, 2019 at 7:51 PM Stephan Ewen <[hidden email]> wrote:
> > >
> > > > Thanks for the pointer David.
> > > >
> > > > I was not aware of this tool and I have no experience with its
> > practical
> > > > impact. For example I would be curious what the effect of this is for
> > > > existing PRs, merge conflicts, etc.
> > > >
> > > > If you want, feel free to start another discuss thread on the
> > > introduction
> > > > of such a tool.
> > > >
> > > > On Sun, Jun 23, 2019 at 6:32 PM David Morávek <[hidden email]>
> wrote:
> > > >
> > > >> I love this kind of unification being pushed forward, the benefit it
> > has
> > > >> on
> > > >> code reviews is enormous. Just my two cents, did you guys think
> about
> > > >> adopting an automatic code formatter for java, the same way as
> Apache
> > > Beam
> > > >> did?
> > > >>
> > > >> It is super easy to use for contributors as they don't need to keep
> > any
> > > >> particular coding style in mind and they can only focus on
> > functionality
> > > >> they want to fix, and it's also great for reviewers, because they
> only
> > > see
> > > >> the important changes. This also eliminates need for any special
> > editor
> > > /
> > > >> checkstyle configs as the code formatting is part of the build
> itself.
> > > >>
> > > >> The one Beam uses is https://github.com/diffplug/spotless with
> > > >> GoogleJavaFormat, it may be worth to look into.
> > > >>
> > > >> Best,
> > > >> David
> > > >>
> > > >> On Fri, Jun 21, 2019 at 4:40 PM Stephan Ewen <[hidden email]>
> > wrote:
> > > >>
> > > >> > Thanks, everyone, for the positive feedback :-)
> > > >> >
> > > >> > @Robert - It probably makes sense to break this down into various
> > > pages,
> > > >> > like PR, general code style guide, Java, component specific
> guides,
> > > >> > formats, etc.
> > > >> >
> > > >> > Best,
> > > >> > Stephan
> > > >> >
> > > >> >
> > > >> > On Fri, Jun 21, 2019 at 4:29 PM Robert Metzger <
> [hidden email]
> > >
> > > >> > wrote:
> > > >> >
> > > >> > > It seems that the discussion around this topic has settled.
> > > >> > >
> > > >> > > I'm going to turn the Google Doc into a markdown file (maybe
> also
> > > >> > multiple,
> > > >> > > I'll try out different things) and then open a pull request for
> > the
> > > >> Flink
> > > >> > > website.
> > > >> > > I'll post a link to the PR here once I'm done.
> > > >> > >
> > > >> > > On Fri, Jun 14, 2019 at 9:36 AM zhijiang <
> > > [hidden email]
> > > >> > > .invalid>
> > > >> > > wrote:
> > > >> > >
> > > >> > > > Thanks for providing this useful guide for benefiting both
> > > >> contributors
> > > >> > > > and committers in consistency.
> > > >> > > >
> > > >> > > > I just reviewed and learned the whole doc which covers a lot
> of
> > > >> > > > information. Wish it further categoried and put onto somewhere
> > for
> > > >> > easily
> > > >> > > > traced future.
> > > >> > > >
> > > >> > > > Best,
> > > >> > > > Zhijiang
> > > >> > > >
> > ------------------------------------------------------------------
> > > >> > > > From:Robert Metzger <[hidden email]>
> > > >> > > > Send Time:2019年6月14日(星期五) 14:24
> > > >> > > > To:dev <[hidden email]>
> > > >> > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality Guide
> > > >> > > >
> > > >> > > > Thanks a lot for putting this together!
> > > >> > > >
> > > >> > > > I'm in the process of reworking the "How to contribute" pages
> > [1]
> > > >> and
> > > >> > I'm
> > > >> > > > happy to add the guide to the Flink website, once the
> discussion
> > > >> here
> > > >> > is
> > > >> > > > over.
> > > >> > > >
> > > >> > > > [1] https://github.com/apache/flink-web/pull/217
> > > >> > > >
> > > >> > > > On Fri, Jun 14, 2019 at 3:21 AM Kurt Young <[hidden email]>
> > > >> wrote:
> > > >> > > >
> > > >> > > > > Big +1 and thanks for preparing this.
> > > >> > > > >
> > > >> > > > > I think wha't more important is making sure most all the
> > > >> contributors
> > > >> > > can
> > > >> > > > > follow
> > > >> > > > > the same guide, a clear document is definitely a great
> start.
> > > >> > > Committers
> > > >> > > > > can
> > > >> > > > > first try to follow the guide by self, and spread the
> standard
> > > >> during
> > > >> > > > code
> > > >> > > > > reviewing.
> > > >> > > > >
> > > >> > > > > Best,
> > > >> > > > > Kurt
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > On Thu, Jun 13, 2019 at 8:28 PM Congxian Qiu <
> > > >> [hidden email]
> > > >> > >
> > > >> > > > > wrote:
> > > >> > > > >
> > > >> > > > > > +1 for this, I think all contributors can benefit from
> this.
> > > >> > > > > >
> > > >> > > > > > Best,
> > > >> > > > > > Congxian
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > Aljoscha Krettek <[hidden email]> 于2019年6月13日周四
> > > 下午8:14写道:
> > > >> > > > > >
> > > >> > > > > > > +1 I think this is a very good effort and should put to
> > rest
> > > >> some
> > > >> > > > > > > back-and-forth discussions on PRs and some differences
> in
> > > >> “style”
> > > >> > > > > between
> > > >> > > > > > > committers. ;-)
> > > >> > > > > > >
> > > >> > > > > > > > On 13. Jun 2019, at 10:21, JingsongLee <
> > > >> > [hidden email]
> > > >> > > > > > .INVALID>
> > > >> > > > > > > wrote:
> > > >> > > > > > > >
> > > >> > > > > > > > big +1, the content is very useful and enlightening.
> > > >> > > > > > > > But it's really too long to look at.
> > > >> > > > > > > > +1 for splitting it and expose it to contributors.
> > > >> > > > > > > >
> > > >> > > > > > > > Even I think it's possible to put its link on the
> > default
> > > >> > > > description
> > > >> > > > > > of
> > > >> > > > > > > > pull request, so that the user has to see it when
> > submits
> > > >> the
> > > >> > > code.
> > > >> > > > > > > >
> > > >> > > > > > > > Best, JingsongLee
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > >
> ------------------------------------------------------------------
> > > >> > > > > > > > From:Piotr Nowojski <[hidden email]>
> > > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03
> > > >> > > > > > > > To:dev <[hidden email]>
> > > >> > > > > > > > Subject:Re: [DISCUSS] Adopting a Code Style and
> Quality
> > > >> Guide
> > > >> > > > > > > >
> > > >> > > > > > > > +1 for it and general content and thank everybody that
> > was
> > > >> > > involved
> > > >> > > > > in
> > > >> > > > > > > creating & writing this down.
> > > >> > > > > > > >
> > > >> > > > > > > > +1 for splitting it up into some easily navigable
> > topics.
> > > >> > > > > > > >
> > > >> > > > > > > > Piotrek
> > > >> > > > > > > >
> > > >> > > > > > > >> On 13 Jun 2019, at 09:54, Stephan Ewen <
> > [hidden email]
> > > >
> > > >> > > wrote:
> > > >> > > > > > > >>
> > > >> > > > > > > >> This should definitely be split up into topics,
> agreed.
> > > >> > > > > > > >> And it should be linked form the "how to contribute"
> > page
> > > >> or
> > > >> > the
> > > >> > > > PR
> > > >> > > > > > > >> template to make contributors aware.
> > > >> > > > > > > >>
> > > >> > > > > > > >> On Thu, Jun 13, 2019 at 9:51 AM Zili Chen <
> > > >> > [hidden email]
> > > >> > > >
> > > >> > > > > > wrote:
> > > >> > > > > > > >>
> > > >> > > > > > > >>> Thanks for creating this guide Stephan. It is also
> > > >> > > > > > > >>> a good start point to internals doc.
> > > >> > > > > > > >>>
> > > >> > > > > > > >>> One suggestion is we could finally separate the
> guide
> > > >> > > > > > > >>> into separated files each of which focus on a
> specific
> > > >> > > > > > > >>> topic. Besides, add the guide to our repository
> should
> > > >> > > > > > > >>> make contributors more aware of it.
> > > >> > > > > > > >>>
> > > >> > > > > > > >>> Best,
> > > >> > > > > > > >>> tison.
> > > >> > > > > > > >>>
> > > >> > > > > > > >>>
> > > >> > > > > > > >>> Jeff Zhang <[hidden email]> 于2019年6月13日周四
> 下午3:35写道:
> > > >> > > > > > > >>>
> > > >> > > > > > > >>>> Thanks for the proposal, Stephan. Big +1 on this.
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>> This is definitely helpful and indispensable for
> > > flink's
> > > >> > code
> > > >> > > > > > quality
> > > >> > > > > > > and
> > > >> > > > > > > >>>> healthy community growth.
> > > >> > > > > > > >>>> It would also benefit downstream project to
> integrate
> > > >> flink
> > > >> > > > > easier.
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>> Till Rohrmann <[hidden email]> 于2019年6月13日周四
> > > >> > 下午3:29写道:
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>>> Thanks for creating this code style and quality
> > guide
> > > >> > > Stephan.
> > > >> > > > I
> > > >> > > > > > > think
> > > >> > > > > > > >>>>> Flink can benefit from such a guide. Thus +1 for
> > > >> > introducing
> > > >> > > > and
> > > >> > > > > > > >>> adhering
> > > >> > > > > > > >>>>> to it.
> > > >> > > > > > > >>>>>
> > > >> > > > > > > >>>>> Cheers,
> > > >> > > > > > > >>>>> Till
> > > >> > > > > > > >>>>>
> > > >> > > > > > > >>>>> On Thu, Jun 13, 2019 at 5:26 AM Bowen Li <
> > > >> > > [hidden email]>
> > > >> > > > > > > wrote:
> > > >> > > > > > > >>>>>
> > > >> > > > > > > >>>>>> Hi Stephan,
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> Definitely a good guide in principle IMO! I
> > > personally
> > > >> > > already
> > > >> > > > > > find
> > > >> > > > > > > >>> it
> > > >> > > > > > > >>>>>> useful in practice to learn from it myself, and
> > also
> > > >> > promote
> > > >> > > > and
> > > >> > > > > > > >>>>> cultivate
> > > >> > > > > > > >>>>>> a better coding habit around by referencing it.
> So
> > > big
> > > >> +1
> > > >> > to
> > > >> > > > > adopt
> > > >> > > > > > > >>> it.
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> Also want to highlight that we need to make real
> > use
> > > of
> > > >> > it,
> > > >> > > > and
> > > >> > > > > > keep
> > > >> > > > > > > >>>>>> ensuring people are aware of its existence.
> Adding
> > it
> > > >> to
> > > >> > > Flink
> > > >> > > > > > > >>> website
> > > >> > > > > > > >>>>>> would be nice. We can also adding noticeable link
> > for
> > > >> it
> > > >> > to
> > > >> > > > the
> > > >> > > > > > > >>>> template
> > > >> > > > > > > >>>>> of
> > > >> > > > > > > >>>>>> github PR (where this guide really matters) to
> get
> > > >> > attention
> > > >> > > > and
> > > >> > > > > > > >>>>> exposure.
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> BTW, what's the plan on how people can raise new
> > > >> > > > > > > coding-style/quality
> > > >> > > > > > > >>>>>> related questions, how to expand and adjust the
> > > content
> > > >> > over
> > > >> > > > > time,
> > > >> > > > > > > >>> how
> > > >> > > > > > > >>>> to
> > > >> > > > > > > >>>>>> inform the community of such updates and changes?
> > > >> Maybe we
> > > >> > > can
> > > >> > > > > use
> > > >> > > > > > > >>> some
> > > >> > > > > > > >>>>>> tags like "[CODING STYLE]" in dev mailing list
> for
> > > >> these
> > > >> > > type
> > > >> > > > of
> > > >> > > > > > > >>>>>> communications? This can be a separate discussion
> > > >> though
> > > >> > if
> > > >> > > we
> > > >> > > > > > wish.
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> All in all, big +1 for adopting it.
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> Bowen
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>> On Wed, Jun 12, 2019 at 12:32 PM Stephan Ewen <
> > > >> > > > [hidden email]
> > > >> > > > > >
> > > >> > > > > > > >>>> wrote:
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>>> Hi all!
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> I want to propose that we try and adopt a code
> > style
> > > >> and
> > > >> > > > > quality
> > > >> > > > > > > >>>> guide.
> > > >> > > > > > > >>>>>> Its
> > > >> > > > > > > >>>>>>> a big endeavor, but I think it's worth trying
> :-)
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> The Apache Flink community and contributor base
> is
> > > >> > growing
> > > >> > > > > quite
> > > >> > > > > > a
> > > >> > > > > > > >>>> bit,
> > > >> > > > > > > >>>>>> new
> > > >> > > > > > > >>>>>>> committers and contributors are coming on board
> > > >> > frequently.
> > > >> > > > > > > >>> Everyone
> > > >> > > > > > > >>>>>> comes
> > > >> > > > > > > >>>>>>> from a different background and brings a
> different
> > > >> level
> > > >> > of
> > > >> > > > > > > >>>> experience.
> > > >> > > > > > > >>>>>>> Different contributors apply different styles,
> and
> > > >> > > > > contributions
> > > >> > > > > > > >>> are
> > > >> > > > > > > >>>>>>> naturally of varying code quality.
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> We are struggling with finding a good balance
> > > between:
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> (1) On the one hand maintaining a certain code
> > > >> standard
> > > >> > > for a
> > > >> > > > > big
> > > >> > > > > > > >>>> and
> > > >> > > > > > > >>>>>>> complex system like Flink, to reduce bugs and
> make
> > > >> future
> > > >> > > > > > > >>>> contributions
> > > >> > > > > > > >>>>>>> feasible (and not impossible due to code
> > > complexity).
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> (2) On the other hand, make contributions
> possible
> > > for
> > > >> > > > > > > >>>> contributors.
> > > >> > > > > > > >>>>>> This
> > > >> > > > > > > >>>>>>> means not guarding everything to the point that
> no
> > > >> > > > > contributions
> > > >> > > > > > > >>> can
> > > >> > > > > > > >>>>> get
> > > >> > > > > > > >>>>>> in
> > > >> > > > > > > >>>>>>> any more.
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> My suggestion to help with this is to define a
> > > >> standard
> > > >> > and
> > > >> > > > > > > >>> document
> > > >> > > > > > > >>>> it
> > > >> > > > > > > >>>>>>> explicitly. It will help to get everyone on the
> > same
> > > >> page
> > > >> > > > what
> > > >> > > > > > the
> > > >> > > > > > > >>>>>>> expectation is, and it helps contributors know
> > what
> > > to
> > > >> > pay
> > > >> > > > > > > >>> attention
> > > >> > > > > > > >>>>> to.
> > > >> > > > > > > >>>>>>> Such a guide should also help make review more
> > > >> efficient,
> > > >> > > > > because
> > > >> > > > > > > >>>>>>> contributors can know up front what reviewers
> will
> > > >> look
> > > >> > at.
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> Over the past weeks, I took a look at the
> current
> > > >> Flink
> > > >> > > code
> > > >> > > > > base
> > > >> > > > > > > >>> and
> > > >> > > > > > > >>>>>>> talked to some committers and contributors and
> > tried
> > > >> to
> > > >> > > come
> > > >> > > > up
> > > >> > > > > > > >>> with
> > > >> > > > > > > >>>> a
> > > >> > > > > > > >>>>>>> proposal that reflects the approaches and
> > standards
> > > >> that
> > > >> > > many
> > > >> > > > > > > >>>>> committers
> > > >> > > > > > > >>>>>>> have adopted lately.
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> This guide is not fix and final, we should
> discuss
> > > it
> > > >> and
> > > >> > > > > expand
> > > >> > > > > > > >>> and
> > > >> > > > > > > >>>>>> adjust
> > > >> > > > > > > >>>>>>> it over time. The guide tries to strike a
> balance
> > > >> between
> > > >> > > too
> > > >> > > > > > much
> > > >> > > > > > > >>>>>> contents
> > > >> > > > > > > >>>>>>> (don't force someone to read a book before
> > > >> contributing)
> > > >> > > and
> > > >> > > > > > being
> > > >> > > > > > > >>>>>>> comprehensive enough. The guide looks long, but
> > much
> > > >> of
> > > >> > it
> > > >> > > > are
> > > >> > > > > > > >>>>> component
> > > >> > > > > > > >>>>>> or
> > > >> > > > > > > >>>>>>> aspect specific parts, like how to deal with new
> > > >> > > > dependencies,
> > > >> > > > > > > >>>>>>> configuration changes, etc.
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> I an curious to hear whether you think such a
> > guide
> > > >> is in
> > > >> > > > > > > >>> principle a
> > > >> > > > > > > >>>>>> good
> > > >> > > > > > > >>>>>>> idea.
> > > >> > > > > > > >>>>>>> If yes, is the one here a good starting point?
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>> Best,
> > > >> > > > > > > >>>>>>> Stephan
> > > >> > > > > > > >>>>>>>
> > > >> > > > > > > >>>>>>
> > > >> > > > > > > >>>>>
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>> --
> > > >> > > > > > > >>>> Best Regards
> > > >> > > > > > > >>>>
> > > >> > > > > > > >>>> Jeff Zhang
> > > >> > > > > > > >>>>
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Adopting a Code Style and Quality Guide

Robert Metzger
Hi all,

I've now merged the guide to the Flink website:
https://flink.apache.org/contributing/code-style-and-quality-preamble.html

I'm looking forward to more pull requests enhancing the guide.


On Tue, Jul 23, 2019 at 7:22 PM Hugo Louro <[hidden email]> wrote:

> Sounds good @stephan! Thanks.
>
> On Tue, Jul 23, 2019 at 10:15 AM Stephan Ewen <[hidden email]> wrote:
>
> > @hugo For your suggestions, I would ask to start a separate discussion
> > thread.
> > I think this mail thread has converged towards merging the initial
> > suggestion as a starting point and refining it later based on new
> > discussions.
> >
> > Best,
> > Stephan
> >
> >
> > On Thu, Jun 27, 2019 at 10:48 PM Hugo Louro <[hidden email]> wrote:
> >
> > > +1. Thanks for working on the guide. It's very thorough and a good
> > resource
> > > to learn good practices from.
> > >
> > > I would like use this thread as a placeholder for a couple of topics
> that
> > > may be deserving of further discussion on different threads:
> > >   - What's the best way to keep track of checkstyle version updates.
> For
> > > instance, currently there is a PR
> > > <https://github.com/apache/flink/pull/8870> proposing checkstyle to be
> > > updated because version 8.12 is no longer supported
> > >  - When classes import shaded dependencies, it is not trivial for
> > IntelliJ
> > > to download and associate sources and javadocs, which makes debugging
> and
> > > navigate the code base harder. I tried installing the version of the
> > > library using maven from the CLI, and associate the sources "manually"
> on
> > > IntelliJ, but it seems it does not work (perhaps IntelliJ issue). Does
> > > anyone know of a good solution? If so, we should added here
> > > <
> > >
> >
> https://ci.apache.org/projects/flink/flink-docs-release-1.8/flinkDev/ide_setup.html#intellij-idea
> > > >.
> > > I can volunteer for that if you tell me how to do it :)
> > > - did the community evaluate naming test methods according to these
> > > conventions <https://stackoverflow.com/a/1594049> ?
> > >
> > > Thanks
> > >
> > > On Mon, Jun 24, 2019 at 11:38 AM Stephan Ewen <[hidden email]>
> wrote:
> > >
> > > > I think it makes sense to also start individual [DISCUSS] threads
> about
> > > > extensions and follow-ups.
> > > > Various suggestions came up, partly as comments in the doc, some as
> > > > questions in other threads.
> > > >
> > > > Examples:
> > > >   - use of null in return types versus Optional versus
> > @Nullable/@Nonnull
> > > >   - initialization of collection sizes
> > > >   - logging
> > > >
> > > > I think these would be best discussed in separate threads each.
> > > > So, for contributors to whom these issues are dear, feel free to
> spawn
> > > > these additional threads.
> > > > (Bear in mind it is close to 1.9 feature freeze time, so please leave
> > > this
> > > > discussions a bit of time so that all community members have a chance
> > to
> > > > participate)
> > > >
> > > >
> > > >
> > > > On Mon, Jun 24, 2019 at 7:51 PM Stephan Ewen <[hidden email]>
> wrote:
> > > >
> > > > > Thanks for the pointer David.
> > > > >
> > > > > I was not aware of this tool and I have no experience with its
> > > practical
> > > > > impact. For example I would be curious what the effect of this is
> for
> > > > > existing PRs, merge conflicts, etc.
> > > > >
> > > > > If you want, feel free to start another discuss thread on the
> > > > introduction
> > > > > of such a tool.
> > > > >
> > > > > On Sun, Jun 23, 2019 at 6:32 PM David Morávek <[hidden email]>
> > wrote:
> > > > >
> > > > >> I love this kind of unification being pushed forward, the benefit
> it
> > > has
> > > > >> on
> > > > >> code reviews is enormous. Just my two cents, did you guys think
> > about
> > > > >> adopting an automatic code formatter for java, the same way as
> > Apache
> > > > Beam
> > > > >> did?
> > > > >>
> > > > >> It is super easy to use for contributors as they don't need to
> keep
> > > any
> > > > >> particular coding style in mind and they can only focus on
> > > functionality
> > > > >> they want to fix, and it's also great for reviewers, because they
> > only
> > > > see
> > > > >> the important changes. This also eliminates need for any special
> > > editor
> > > > /
> > > > >> checkstyle configs as the code formatting is part of the build
> > itself.
> > > > >>
> > > > >> The one Beam uses is https://github.com/diffplug/spotless with
> > > > >> GoogleJavaFormat, it may be worth to look into.
> > > > >>
> > > > >> Best,
> > > > >> David
> > > > >>
> > > > >> On Fri, Jun 21, 2019 at 4:40 PM Stephan Ewen <[hidden email]>
> > > wrote:
> > > > >>
> > > > >> > Thanks, everyone, for the positive feedback :-)
> > > > >> >
> > > > >> > @Robert - It probably makes sense to break this down into
> various
> > > > pages,
> > > > >> > like PR, general code style guide, Java, component specific
> > guides,
> > > > >> > formats, etc.
> > > > >> >
> > > > >> > Best,
> > > > >> > Stephan
> > > > >> >
> > > > >> >
> > > > >> > On Fri, Jun 21, 2019 at 4:29 PM Robert Metzger <
> > [hidden email]
> > > >
> > > > >> > wrote:
> > > > >> >
> > > > >> > > It seems that the discussion around this topic has settled.
> > > > >> > >
> > > > >> > > I'm going to turn the Google Doc into a markdown file (maybe
> > also
> > > > >> > multiple,
> > > > >> > > I'll try out different things) and then open a pull request
> for
> > > the
> > > > >> Flink
> > > > >> > > website.
> > > > >> > > I'll post a link to the PR here once I'm done.
> > > > >> > >
> > > > >> > > On Fri, Jun 14, 2019 at 9:36 AM zhijiang <
> > > > [hidden email]
> > > > >> > > .invalid>
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > Thanks for providing this useful guide for benefiting both
> > > > >> contributors
> > > > >> > > > and committers in consistency.
> > > > >> > > >
> > > > >> > > > I just reviewed and learned the whole doc which covers a lot
> > of
> > > > >> > > > information. Wish it further categoried and put onto
> somewhere
> > > for
> > > > >> > easily
> > > > >> > > > traced future.
> > > > >> > > >
> > > > >> > > > Best,
> > > > >> > > > Zhijiang
> > > > >> > > >
> > > ------------------------------------------------------------------
> > > > >> > > > From:Robert Metzger <[hidden email]>
> > > > >> > > > Send Time:2019年6月14日(星期五) 14:24
> > > > >> > > > To:dev <[hidden email]>
> > > > >> > > > Subject:Re: [DISCUSS] Adopting a Code Style and Quality
> Guide
> > > > >> > > >
> > > > >> > > > Thanks a lot for putting this together!
> > > > >> > > >
> > > > >> > > > I'm in the process of reworking the "How to contribute"
> pages
> > > [1]
> > > > >> and
> > > > >> > I'm
> > > > >> > > > happy to add the guide to the Flink website, once the
> > discussion
> > > > >> here
> > > > >> > is
> > > > >> > > > over.
> > > > >> > > >
> > > > >> > > > [1] https://github.com/apache/flink-web/pull/217
> > > > >> > > >
> > > > >> > > > On Fri, Jun 14, 2019 at 3:21 AM Kurt Young <
> [hidden email]>
> > > > >> wrote:
> > > > >> > > >
> > > > >> > > > > Big +1 and thanks for preparing this.
> > > > >> > > > >
> > > > >> > > > > I think wha't more important is making sure most all the
> > > > >> contributors
> > > > >> > > can
> > > > >> > > > > follow
> > > > >> > > > > the same guide, a clear document is definitely a great
> > start.
> > > > >> > > Committers
> > > > >> > > > > can
> > > > >> > > > > first try to follow the guide by self, and spread the
> > standard
> > > > >> during
> > > > >> > > > code
> > > > >> > > > > reviewing.
> > > > >> > > > >
> > > > >> > > > > Best,
> > > > >> > > > > Kurt
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > On Thu, Jun 13, 2019 at 8:28 PM Congxian Qiu <
> > > > >> [hidden email]
> > > > >> > >
> > > > >> > > > > wrote:
> > > > >> > > > >
> > > > >> > > > > > +1 for this, I think all contributors can benefit from
> > this.
> > > > >> > > > > >
> > > > >> > > > > > Best,
> > > > >> > > > > > Congxian
> > > > >> > > > > >
> > > > >> > > > > >
> > > > >> > > > > > Aljoscha Krettek <[hidden email]> 于2019年6月13日周四
> > > > 下午8:14写道:
> > > > >> > > > > >
> > > > >> > > > > > > +1 I think this is a very good effort and should put
> to
> > > rest
> > > > >> some
> > > > >> > > > > > > back-and-forth discussions on PRs and some differences
> > in
> > > > >> “style”
> > > > >> > > > > between
> > > > >> > > > > > > committers. ;-)
> > > > >> > > > > > >
> > > > >> > > > > > > > On 13. Jun 2019, at 10:21, JingsongLee <
> > > > >> > [hidden email]
> > > > >> > > > > > .INVALID>
> > > > >> > > > > > > wrote:
> > > > >> > > > > > > >
> > > > >> > > > > > > > big +1, the content is very useful and enlightening.
> > > > >> > > > > > > > But it's really too long to look at.
> > > > >> > > > > > > > +1 for splitting it and expose it to contributors.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Even I think it's possible to put its link on the
> > > default
> > > > >> > > > description
> > > > >> > > > > > of
> > > > >> > > > > > > > pull request, so that the user has to see it when
> > > submits
> > > > >> the
> > > > >> > > code.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Best, JingsongLee
> > > > >> > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > >
> > > > >> > >
> > ------------------------------------------------------------------
> > > > >> > > > > > > > From:Piotr Nowojski <[hidden email]>
> > > > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03
> > > > >> > > > > > > > To:dev <[hidden email]>
> > > > >> > > > > > > > Subject:Re: [DISCUSS] Adopting a Code Style and
> > Quality
> > > > >> Guide
> > > > >> > > > > > > >
> > > > >> > > > > > > > +1 for it and general content and thank everybody
> that
> > > was
> > > > >> > > involved
> > > > >> > > > > in
> > > > >> > > > > > > creating & writing this down.
> > > > >> > > > > > > >
> > > > >> > > > > > > > +1 for splitting it up into some easily navigable
> > > topics.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Piotrek
> > > > >> > > > > > > >
> > > > >> > > > > > > >> On 13 Jun 2019, at 09:54, Stephan Ewen <
> > > [hidden email]
> > > > >
> > > > >> > > wrote:
> > > > >> > > > > > > >>
> > > > >> > > > > > > >> This should definitely be split up into topics,
> > agreed.
> > > > >> > > > > > > >> And it should be linked form the "how to
> contribute"
> > > page
> > > > >> or
> > > > >> > the
> > > > >> > > > PR
> > > > >> > > > > > > >> template to make contributors aware.
> > > > >> > > > > > > >>
> > > > >> > > > > > > >> On Thu, Jun 13, 2019 at 9:51 AM Zili Chen <
> > > > >> > [hidden email]
> > > > >> > > >
> > > > >> > > > > > wrote:
> > > > >> > > > > > > >>
> > > > >> > > > > > > >>> Thanks for creating this guide Stephan. It is also
> > > > >> > > > > > > >>> a good start point to internals doc.
> > > > >> > > > > > > >>>
> > > > >> > > > > > > >>> One suggestion is we could finally separate the
> > guide
> > > > >> > > > > > > >>> into separated files each of which focus on a
> > specific
> > > > >> > > > > > > >>> topic. Besides, add the guide to our repository
> > should
> > > > >> > > > > > > >>> make contributors more aware of it.
> > > > >> > > > > > > >>>
> > > > >> > > > > > > >>> Best,
> > > > >> > > > > > > >>> tison.
> > > > >> > > > > > > >>>
> > > > >> > > > > > > >>>
> > > > >> > > > > > > >>> Jeff Zhang <[hidden email]> 于2019年6月13日周四
> > 下午3:35写道:
> > > > >> > > > > > > >>>
> > > > >> > > > > > > >>>> Thanks for the proposal, Stephan. Big +1 on this.
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>> This is definitely helpful and indispensable for
> > > > flink's
> > > > >> > code
> > > > >> > > > > > quality
> > > > >> > > > > > > and
> > > > >> > > > > > > >>>> healthy community growth.
> > > > >> > > > > > > >>>> It would also benefit downstream project to
> > integrate
> > > > >> flink
> > > > >> > > > > easier.
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>> Till Rohrmann <[hidden email]>
> 于2019年6月13日周四
> > > > >> > 下午3:29写道:
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>>> Thanks for creating this code style and quality
> > > guide
> > > > >> > > Stephan.
> > > > >> > > > I
> > > > >> > > > > > > think
> > > > >> > > > > > > >>>>> Flink can benefit from such a guide. Thus +1 for
> > > > >> > introducing
> > > > >> > > > and
> > > > >> > > > > > > >>> adhering
> > > > >> > > > > > > >>>>> to it.
> > > > >> > > > > > > >>>>>
> > > > >> > > > > > > >>>>> Cheers,
> > > > >> > > > > > > >>>>> Till
> > > > >> > > > > > > >>>>>
> > > > >> > > > > > > >>>>> On Thu, Jun 13, 2019 at 5:26 AM Bowen Li <
> > > > >> > > [hidden email]>
> > > > >> > > > > > > wrote:
> > > > >> > > > > > > >>>>>
> > > > >> > > > > > > >>>>>> Hi Stephan,
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> Definitely a good guide in principle IMO! I
> > > > personally
> > > > >> > > already
> > > > >> > > > > > find
> > > > >> > > > > > > >>> it
> > > > >> > > > > > > >>>>>> useful in practice to learn from it myself, and
> > > also
> > > > >> > promote
> > > > >> > > > and
> > > > >> > > > > > > >>>>> cultivate
> > > > >> > > > > > > >>>>>> a better coding habit around by referencing it.
> > So
> > > > big
> > > > >> +1
> > > > >> > to
> > > > >> > > > > adopt
> > > > >> > > > > > > >>> it.
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> Also want to highlight that we need to make
> real
> > > use
> > > > of
> > > > >> > it,
> > > > >> > > > and
> > > > >> > > > > > keep
> > > > >> > > > > > > >>>>>> ensuring people are aware of its existence.
> > Adding
> > > it
> > > > >> to
> > > > >> > > Flink
> > > > >> > > > > > > >>> website
> > > > >> > > > > > > >>>>>> would be nice. We can also adding noticeable
> link
> > > for
> > > > >> it
> > > > >> > to
> > > > >> > > > the
> > > > >> > > > > > > >>>> template
> > > > >> > > > > > > >>>>> of
> > > > >> > > > > > > >>>>>> github PR (where this guide really matters) to
> > get
> > > > >> > attention
> > > > >> > > > and
> > > > >> > > > > > > >>>>> exposure.
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> BTW, what's the plan on how people can raise
> new
> > > > >> > > > > > > coding-style/quality
> > > > >> > > > > > > >>>>>> related questions, how to expand and adjust the
> > > > content
> > > > >> > over
> > > > >> > > > > time,
> > > > >> > > > > > > >>> how
> > > > >> > > > > > > >>>> to
> > > > >> > > > > > > >>>>>> inform the community of such updates and
> changes?
> > > > >> Maybe we
> > > > >> > > can
> > > > >> > > > > use
> > > > >> > > > > > > >>> some
> > > > >> > > > > > > >>>>>> tags like "[CODING STYLE]" in dev mailing list
> > for
> > > > >> these
> > > > >> > > type
> > > > >> > > > of
> > > > >> > > > > > > >>>>>> communications? This can be a separate
> discussion
> > > > >> though
> > > > >> > if
> > > > >> > > we
> > > > >> > > > > > wish.
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> All in all, big +1 for adopting it.
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> Bowen
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>> On Wed, Jun 12, 2019 at 12:32 PM Stephan Ewen <
> > > > >> > > > [hidden email]
> > > > >> > > > > >
> > > > >> > > > > > > >>>> wrote:
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>>> Hi all!
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> I want to propose that we try and adopt a code
> > > style
> > > > >> and
> > > > >> > > > > quality
> > > > >> > > > > > > >>>> guide.
> > > > >> > > > > > > >>>>>> Its
> > > > >> > > > > > > >>>>>>> a big endeavor, but I think it's worth trying
> > :-)
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> The Apache Flink community and contributor
> base
> > is
> > > > >> > growing
> > > > >> > > > > quite
> > > > >> > > > > > a
> > > > >> > > > > > > >>>> bit,
> > > > >> > > > > > > >>>>>> new
> > > > >> > > > > > > >>>>>>> committers and contributors are coming on
> board
> > > > >> > frequently.
> > > > >> > > > > > > >>> Everyone
> > > > >> > > > > > > >>>>>> comes
> > > > >> > > > > > > >>>>>>> from a different background and brings a
> > different
> > > > >> level
> > > > >> > of
> > > > >> > > > > > > >>>> experience.
> > > > >> > > > > > > >>>>>>> Different contributors apply different styles,
> > and
> > > > >> > > > > contributions
> > > > >> > > > > > > >>> are
> > > > >> > > > > > > >>>>>>> naturally of varying code quality.
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> We are struggling with finding a good balance
> > > > between:
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> (1) On the one hand maintaining a certain code
> > > > >> standard
> > > > >> > > for a
> > > > >> > > > > big
> > > > >> > > > > > > >>>> and
> > > > >> > > > > > > >>>>>>> complex system like Flink, to reduce bugs and
> > make
> > > > >> future
> > > > >> > > > > > > >>>> contributions
> > > > >> > > > > > > >>>>>>> feasible (and not impossible due to code
> > > > complexity).
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> (2) On the other hand, make contributions
> > possible
> > > > for
> > > > >> > > > > > > >>>> contributors.
> > > > >> > > > > > > >>>>>> This
> > > > >> > > > > > > >>>>>>> means not guarding everything to the point
> that
> > no
> > > > >> > > > > contributions
> > > > >> > > > > > > >>> can
> > > > >> > > > > > > >>>>> get
> > > > >> > > > > > > >>>>>> in
> > > > >> > > > > > > >>>>>>> any more.
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> My suggestion to help with this is to define a
> > > > >> standard
> > > > >> > and
> > > > >> > > > > > > >>> document
> > > > >> > > > > > > >>>> it
> > > > >> > > > > > > >>>>>>> explicitly. It will help to get everyone on
> the
> > > same
> > > > >> page
> > > > >> > > > what
> > > > >> > > > > > the
> > > > >> > > > > > > >>>>>>> expectation is, and it helps contributors know
> > > what
> > > > to
> > > > >> > pay
> > > > >> > > > > > > >>> attention
> > > > >> > > > > > > >>>>> to.
> > > > >> > > > > > > >>>>>>> Such a guide should also help make review more
> > > > >> efficient,
> > > > >> > > > > because
> > > > >> > > > > > > >>>>>>> contributors can know up front what reviewers
> > will
> > > > >> look
> > > > >> > at.
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> Over the past weeks, I took a look at the
> > current
> > > > >> Flink
> > > > >> > > code
> > > > >> > > > > base
> > > > >> > > > > > > >>> and
> > > > >> > > > > > > >>>>>>> talked to some committers and contributors and
> > > tried
> > > > >> to
> > > > >> > > come
> > > > >> > > > up
> > > > >> > > > > > > >>> with
> > > > >> > > > > > > >>>> a
> > > > >> > > > > > > >>>>>>> proposal that reflects the approaches and
> > > standards
> > > > >> that
> > > > >> > > many
> > > > >> > > > > > > >>>>> committers
> > > > >> > > > > > > >>>>>>> have adopted lately.
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> This guide is not fix and final, we should
> > discuss
> > > > it
> > > > >> and
> > > > >> > > > > expand
> > > > >> > > > > > > >>> and
> > > > >> > > > > > > >>>>>> adjust
> > > > >> > > > > > > >>>>>>> it over time. The guide tries to strike a
> > balance
> > > > >> between
> > > > >> > > too
> > > > >> > > > > > much
> > > > >> > > > > > > >>>>>> contents
> > > > >> > > > > > > >>>>>>> (don't force someone to read a book before
> > > > >> contributing)
> > > > >> > > and
> > > > >> > > > > > being
> > > > >> > > > > > > >>>>>>> comprehensive enough. The guide looks long,
> but
> > > much
> > > > >> of
> > > > >> > it
> > > > >> > > > are
> > > > >> > > > > > > >>>>> component
> > > > >> > > > > > > >>>>>> or
> > > > >> > > > > > > >>>>>>> aspect specific parts, like how to deal with
> new
> > > > >> > > > dependencies,
> > > > >> > > > > > > >>>>>>> configuration changes, etc.
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> I an curious to hear whether you think such a
> > > guide
> > > > >> is in
> > > > >> > > > > > > >>> principle a
> > > > >> > > > > > > >>>>>> good
> > > > >> > > > > > > >>>>>>> idea.
> > > > >> > > > > > > >>>>>>> If yes, is the one here a good starting point?
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>> Best,
> > > > >> > > > > > > >>>>>>> Stephan
> > > > >> > > > > > > >>>>>>>
> > > > >> > > > > > > >>>>>>
> > > > >> > > > > > > >>>>>
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>> --
> > > > >> > > > > > > >>>> Best Regards
> > > > >> > > > > > > >>>>
> > > > >> > > > > > > >>>> Jeff Zhang
> > > > >> > > > > > > >>>>
> > > > >> > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > >
> > >
> >
>
12