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 <hmclo...@gmail.com> wrote: > Sounds good @stephan! Thanks. > > On Tue, Jul 23, 2019 at 10:15 AM Stephan Ewen <se...@apache.org> 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 <hmclo...@gmail.com> 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 <se...@apache.org> > 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 <se...@apache.org> > 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 <d...@apache.org> > > 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 <se...@apache.org> > > > 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 < > > rmetz...@apache.org > > > > > > > > >> > 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 < > > > > wangzhijiang...@aliyun.com > > > > >> > > .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 <rmetz...@apache.org> > > > > >> > > > Send Time:2019年6月14日(星期五) 14:24 > > > > >> > > > To:dev <dev@flink.apache.org> > > > > >> > > > 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 < > ykt...@gmail.com> > > > > >> 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 < > > > > >> qcx978132...@gmail.com > > > > >> > > > > > > >> > > > > wrote: > > > > >> > > > > > > > > >> > > > > > +1 for this, I think all contributors can benefit from > > this. > > > > >> > > > > > > > > > >> > > > > > Best, > > > > >> > > > > > Congxian > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > Aljoscha Krettek <aljos...@apache.org> 于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 < > > > > >> > lzljs3620...@aliyun.com > > > > >> > > > > > .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 <pi...@ververica.com> > > > > >> > > > > > > > Send Time:2019年6月13日(星期四) 16:03 > > > > >> > > > > > > > To:dev <dev@flink.apache.org> > > > > >> > > > > > > > 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 < > > > se...@apache.org > > > > > > > > > >> > > 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 < > > > > >> > wander4...@gmail.com > > > > >> > > > > > > > >> > > > > > 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 <zjf...@gmail.com> 于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 <trohrm...@apache.org> > 于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 < > > > > >> > > bowenl...@gmail.com> > > > > >> > > > > > > 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 < > > > > >> > > > se...@apache.org > > > > >> > > > > > > > > > >> > > > > > > >>>> 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 > > > > >> > > > > > > >>>> > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > >
