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 <rmetz...@apache.org> 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 <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 >> > >> > > > > > > >>>> >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > > >> > >> > > >> > >> > >> > >> >> > > >> > >> >
