@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 > > >> > > > > > > >>>> > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > >
