Return-Path: X-Original-To: apmail-flink-dev-archive@www.apache.org Delivered-To: apmail-flink-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E002618DF0 for ; Thu, 8 Oct 2015 11:17:05 +0000 (UTC) Received: (qmail 62590 invoked by uid 500); 8 Oct 2015 11:16:59 -0000 Delivered-To: apmail-flink-dev-archive@flink.apache.org Received: (qmail 62530 invoked by uid 500); 8 Oct 2015 11:16:59 -0000 Mailing-List: contact dev-help@flink.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flink.apache.org Delivered-To: mailing list dev@flink.apache.org Received: (qmail 62519 invoked by uid 99); 8 Oct 2015 11:16:59 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 08 Oct 2015 11:16:59 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id E8347C4A56 for ; Thu, 8 Oct 2015 11:16:58 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.001 X-Spam-Level: * X-Spam-Status: No, score=1.001 tagged_above=-999 required=6.31 tests=[KAM_LAZY_DOMAIN_SECURITY=1, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id TY-nJbKoe_WS for ; Thu, 8 Oct 2015 11:16:54 +0000 (UTC) Received: from mx2.mailbox.org (mx2.mailbox.org [80.241.60.215]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with ESMTPS id 2D541439BA for ; Thu, 8 Oct 2015 11:16:54 +0000 (UTC) Received: from smtp1.mailbox.org (smtp1.mailbox.org [80.241.60.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx2.mailbox.org (Postfix) with ESMTPS id 2BA7E4343F for ; Thu, 8 Oct 2015 13:16:53 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by hefe.heinlein-support.de (hefe.heinlein-support.de [91.198.250.172]) (amavisd-new, port 10030) with ESMTP id ENhVjqYmEbNm for ; Thu, 8 Oct 2015 13:16:50 +0200 (CEST) Message-ID: <5616508C.2050801@apache.org> Date: Thu, 08 Oct 2015 13:16:28 +0200 From: "Matthias J. Sax" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: dev@flink.apache.org Subject: Re: Extending and improving our "How to contribute" page References: <0024A2DC-5EDD-47E5-9CF7-F14FA5FC0578@apache.org> <34603366-4217-4C4E-93F6-49969A83F0E1@apache.org> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NlmHuTvExx3VxCRkJtOkVtl1b2cgeXeH7" --NlmHuTvExx3VxCRkJtOkVtl1b2cgeXeH7 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable +1 Great! On 10/08/2015 01:14 PM, Fabian Hueske wrote: > Hi everybody, >=20 > I merged our new contribution guidelines a few minutes ago. >=20 > I'd like to emphasize that these rules do not have any effect, if nobod= y > follows them. > So please follow our contribution rules and make others aware of them a= s > well. >=20 > Specifically > - pay attention that all PRs are backed by a JIRA and ask to create a J= IRA > if that is not the case > - early discuss whether a feature request is valid (before code is > contributed) to avoid frustrating late rejections of PRs. > - request, provide, and discuss design docs for sensible contributions = to > avoid major redesigns / rejections of PRs. >=20 > Thank you, > Fabian >=20 > 2015-10-07 10:16 GMT+02:00 Fabian Hueske : >=20 >> Thanks for the feedback everybody. >> I updated the PR and would like to merge it later today if there are n= o >> more comments. >> >> Cheers, Fabian >> >> >> 2015-10-05 14:09 GMT+02:00 Fabian Hueske : >> >>> Hi, >>> >>> I opened a PR with the discussed changes [1]. >>> Please review, give feedback, and suggest changes. >>> >>> Cheers, Fabian >>> >>> [1] https://github.com/apache/flink-web/pull/11 >>> >>> >>> 2015-09-28 18:02 GMT+02:00 Fabian Hueske : >>> >>>> @Chiwan, sure. Will do that. Thanks for pointing it out :-) >>>> >>>> 2015-09-28 18:00 GMT+02:00 Chiwan Park : >>>> >>>>> @Fabian, Could you cover FLINK-2712 in your pull request? I think t= hat >>>>> it would be better than split pull request. >>>>> >>>>> Regards, >>>>> Chiwan Park >>>>> >>>>>> On Sep 28, 2015, at 4:51 PM, Fabian Hueske wro= te: >>>>>> >>>>>> Thanks everybody for the discussion. >>>>>> I'll prepare a pull request to update the "How to contribute" and >>>>> "Coding >>>>>> Guidelines". >>>>>> >>>>>> Thanks, >>>>>> Fabian >>>>>> >>>>>> 2015-09-26 9:06 GMT+02:00 Maximilian Michels : >>>>>> >>>>>>> Hi Fabian, >>>>>>> >>>>>>> This is a very important topic. Thanks for starting the discussio= n. >>>>>>> >>>>>>> 1) JIRA discussion >>>>>>> >>>>>>> Absolutely. No new feature should be introduced without a discuss= ion. >>>>>>> Frankly, I see the problem that sometimes discussions only come u= p >>>>>>> when the pull request has been opened. However, this can be overc= ome >>>>>>> by the design document. >>>>>>> >>>>>>> 2) Design document >>>>>>> >>>>>>> +1 for the document. It increases transparency but also helps the= >>>>>>> contributor to think his idea through before starting to code. Th= e >>>>>>> document could also be written directly in JIRA. That way, it is = more >>>>>>> accessible. JIRA offers mark up; even images can be attached and >>>>>>> displayed in the JIRA description. >>>>>>> >>>>>>> I'd like to propose another section "Limitations" for the design >>>>>>> document. Breaking API changes should also be listed on a special= >>>>> Wiki >>>>>>> page. >>>>>>> >>>>>>> 3) Coding style >>>>>>> >>>>>>> In addition to updating the document, do we want to enforce codin= g >>>>>>> styles also by adding new Maven Checkstyle rules? IMHO strict rul= es >>>>>>> could cause more annoyances than they actually contribute to the >>>>>>> readability of the code. Perhaps this should be discussed in a >>>>>>> separate thread. >>>>>>> >>>>>>> +1 for collecting common problems and design patterns to include = them >>>>>>> in the document. I was thinking, that we should also cover some o= f >>>>> the >>>>>>> features of tools and dependencies we heavily use, e.g. Travis, >>>>>>> Mockito, Guava, Log4j, FlinkMiniCluster, Unit testing vs IT cases= , >>>>>>> etc. >>>>>>> >>>>>>> 4 ) Restructuring the how to contribute guide >>>>>>> >>>>>>> Good idea to have a meta document that explains how contributing >>>>> works >>>>>>> in general, and another document for technical things. >>>>>>> >>>>>>> >>>>>>> Cheers, >>>>>>> Max >>>>>>> >>>>>>> >>>>>>> On Thu, Sep 24, 2015 at 2:53 PM, Fabian Hueske >>>>> wrote: >>>>>>>> >>>>>>>> Thanks everybody for feedback and comments. >>>>>>>> >>>>>>>> Regarding 1) and 2): >>>>>>>> >>>>>>>> I like the idea of keeping the discussion of new features and >>>>>>> improvements >>>>>>>> in JIRA as Kostas proposed. >>>>>>>> Our coding guidelines [1] already request a JIRA issue for each = pull >>>>>>>> request. >>>>>>>> >>>>>>>> How about we highlight this requirement more prominently and fol= low >>>>> this >>>>>>>> rule more strict from now on. >>>>>>>> JIRA issues for new features and improvements should clearly >>>>> specify the >>>>>>>> scope and requirements for the new feature / improvement. >>>>>>>> The level of detail is up to the reporter of the issue, but the >>>>> community >>>>>>>> can request more detail or change the scope and requirements by >>>>>>> discussion. >>>>>>>> When a JIRA issue for a new feature or improvement is opened, th= e >>>>>>> community >>>>>>>> can start a discussion whether the feature is desirable for Flin= k >>>>> or not. >>>>>>>> Any contributor (including the reporter) can also attach a >>>>>>>> "design-doc-requested" label to the issue. A design document can= be >>>>>>>> proposed by anybody, including the reporter or assignee of the J= IRA >>>>>>> issue. >>>>>>>> However, the issue cannot be resolved and a corresponding PR not= be >>>>>>> merged >>>>>>>> before a design document has been accepted by lazy consensus. >>>>> Hence, an >>>>>>>> assignee should propose a design doc before starting to code to >>>>> avoid >>>>>>> major >>>>>>>> redesigns of the implementation. >>>>>>>> >>>>>>>> This way it is up to the community when to start a discussion ab= out >>>>>>> whether >>>>>>>> a feature request is accepted or to request a design document. W= e >>>>> can >>>>>>> make >>>>>>>> design documents mandatory for changes that touch the public API= =2E >>>>>>>> >>>>>>>> Regarding 3): >>>>>>>> >>>>>>>> I agree with Vasia, that we should collect suggestions for commo= n >>>>>>> patterns >>>>>>>> and also continuously update the coding guidelines. >>>>>>>> @Henry, I had best practices (exception handling, tests, etc.) i= n >>>>> mind. >>>>>>>> Syntactic code style is important as well, but we should have a >>>>> separate >>>>>>>> discussion about that, IMO. >>>>>>>> >>>>>>>> Proposal for a design document template: >>>>>>>> >>>>>>>> - Overview of general approach >>>>>>>> - API changes (changed interfaces, new / deprecated configuratio= n >>>>>>>> parameters, changed behavior) >>>>>>>> - Main components and classes to touch >>>>>>>> >>>>>>>> Cheers, >>>>>>>> Fabian >>>>>>>> >>>>>>>> [1] http://flink.apache.org/coding-guidelines.html >>>>>>>> >>>>>>>> >>>>>>>> 2015-09-24 10:52 GMT+02:00 Chiwan Park : >>>>>>>> >>>>>>>>> Thanks Fabian for starting the discussion. >>>>>>>>> >>>>>>>>> +1 for overall approach. >>>>>>>>> >>>>>>>>> About (1), expressing that consensus must be required for new >>>>> feature >>>>>>> in >>>>>>>>> =E2=80=9CHow to contribute=E2=80=9D page is very nice. Some pul= l requests were sent >>>>>>> without >>>>>>>>> consensus. The contributors had to rewrote their pull requests.= >>>>>>>>> >>>>>>>>> Agree with (2), (3) and (4). >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Chiwan Park >>>>>>>>> >>>>>>>>>> On Sep 24, 2015, at 2:23 AM, Henry Saputra < >>>>> henry.saputra@gmail.com> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> Thanks again, Fabian for starting the discussions. >>>>>>>>>> >>>>>>>>>> For (1) and (2) I think it is good idea and will help people t= o >>>>>>>>>> understand and follow the author thought process. >>>>>>>>>> Following up with Stephan's reply, some new features solutions= >>>>> could >>>>>>>>>> be explained thoroughly in the PR descriptions but some requir= es >>>>>>>>>> additional reviews of the proposed design. >>>>>>>>>> I like the idea of using tag in JIRA whether new features shou= ld >>>>> or >>>>>>>>>> should not being accompanied by design document. >>>>>>>>>> >>>>>>>>>> Agree with (3) and (4). >>>>>>>>>> As for (3) are you thinking about more of style of code syntax= via >>>>>>>>>> checkstyle updates, or best practices in term of no mutable st= ate >>>>> if >>>>>>>>>> possible, throw precise Exception if possible for interfaces, >>>>> etc. ? >>>>>>>>>> >>>>>>>>>> - Henry >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Wed, Sep 23, 2015 at 9:31 AM, Stephan Ewen >>>>>>> wrote: >>>>>>>>>>> Thanks, Fabian for driving this! >>>>>>>>>>> >>>>>>>>>>> I agree with your points. >>>>>>>>>>> >>>>>>>>>>> Concerning Vasia's comment to not raise the bar too high: >>>>>>>>>>> That is true, the requirements should be reasonable. We can >>>>>>> definitely >>>>>>>>> tag >>>>>>>>>>> issues as "simple" which means they do not require a design >>>>>>> document. >>>>>>>>> That >>>>>>>>>>> should be more for new features and needs not be very detaile= d. >>>>>>>>>>> >>>>>>>>>>> We could also make the inverse, meaning we explicitly tag cer= tain >>>>>>>>> issues as >>>>>>>>>>> "requires design document". >>>>>>>>>>> >>>>>>>>>>> Greetings, >>>>>>>>>>> Stephan >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Wed, Sep 23, 2015 at 5:05 PM, Vasiliki Kalavri < >>>>>>>>> vasilikikalavri@gmail.com >>>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi, >>>>>>>>>>>> >>>>>>>>>>>> I agree with you Fabian. Clarifying these issues in the "How= to >>>>>>>>> Contribute" >>>>>>>>>>>> guide will save lots of time both to reviewers and contribut= ors. >>>>>>> It is >>>>>>>>> a >>>>>>>>>>>> really disappointing situation when someone spends time >>>>>>> implementing >>>>>>>>>>>> something and their PR ends up being rejected because either= the >>>>>>>>> feature >>>>>>>>>>>> was not needed or the implementation details were never agre= ed >>>>> on. >>>>>>>>>>>> >>>>>>>>>>>> That said, I think we should also make sure that we don't ra= ise >>>>> the >>>>>>>>> bar too >>>>>>>>>>>> high for simple contributions. >>>>>>>>>>>> >>>>>>>>>>>> Regarding (1) and (2), I think we should clarify what kind o= f >>>>>>>>>>>> additions/changes require this process to be followed. e.g. = do >>>>> we >>>>>>> need >>>>>>>>> to >>>>>>>>>>>> discuss additions for which JIRAs already exist? Ideas descr= ibed >>>>>>> in the >>>>>>>>>>>> roadmaps? Adding a new algorithm to Gelly/Flink-ML? >>>>>>>>>>>> >>>>>>>>>>>> Regarding (3), maybe we can all suggest some examples/patter= ns >>>>> that >>>>>>>>> we've >>>>>>>>>>>> seen when reviewing PRs and then choose the most common (or >>>>> all). >>>>>>>>>>>> >>>>>>>>>>>> (4) sounds good to me. >>>>>>>>>>>> >>>>>>>>>>>> Cheers, >>>>>>>>>>>> Vasia. >>>>>>>>>>>> >>>>>>>>>>>> On 23 September 2015 at 15:08, Kostas Tzoumas < >>>>> ktzoumas@apache.org >>>>>>>> >>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Big +1. >>>>>>>>>>>>> >>>>>>>>>>>>> For (1), a discussion in JIRA would also be an option IMO >>>>>>>>>>>>> >>>>>>>>>>>>> For (2), let us come up with few examples on what constitut= es a >>>>>>>>> feature >>>>>>>>>>>>> that needs a design doc, and what should be in the doc (IMO= >>>>>>>>>>>>> architecture/general approach, components touched, interfac= es >>>>>>> changed) >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Sep 23, 2015 at 2:24 PM, Fabian Hueske < >>>>> fhueske@gmail.com >>>>>>>> >>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi everybody, >>>>>>>>>>>>>> >>>>>>>>>>>>>> I guess we all have noticed that the Flink community is >>>>> quickly >>>>>>>>> growing >>>>>>>>>>>>> and >>>>>>>>>>>>>> more and more contributions are coming in. Recently, a few= >>>>>>>>>>>> contributions >>>>>>>>>>>>>> proposed new features without being discussed on the maili= ng >>>>>>> list. >>>>>>>>> Some >>>>>>>>>>>>> of >>>>>>>>>>>>>> these contributions were not accepted in the end. In other= >>>>> cases, >>>>>>>>> pull >>>>>>>>>>>>>> requests had to be heavily reworked because the approach t= aken >>>>>>> was >>>>>>>>> not >>>>>>>>>>>>> the >>>>>>>>>>>>>> best one. These are situations which should be avoided bec= ause >>>>>>> both >>>>>>>>> the >>>>>>>>>>>>>> contributor as well as the person who reviewed the >>>>> contribution >>>>>>>>>>>> invested >>>>>>>>>>>>> a >>>>>>>>>>>>>> lot of time for nothing. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I had a look at our =E2=80=9CHow to contribute=E2=80=9D an= d =E2=80=9CCoding guideline=E2=80=9D >>>>>>> pages >>>>>>>>>>>> and >>>>>>>>>>>>>> think, we can improve them. I see basically two issues: >>>>>>>>>>>>>> >>>>>>>>>>>>>> 1. The documents do not explain how to propose and discuss= new >>>>>>>>>>>> features >>>>>>>>>>>>>> and improvements. >>>>>>>>>>>>>> 2. The documents are quite technical and the structure cou= ld >>>>> be >>>>>>>>>>>>> improved, >>>>>>>>>>>>>> IMO. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I would like to improve these pages and propose the follow= ing >>>>>>>>>>>> additions: >>>>>>>>>>>>>> >>>>>>>>>>>>>> 1. Request contributors and committers to start discussion= s on >>>>>>> the >>>>>>>>>>>>>> mailing list for new features. This discussion should help= to >>>>>>> figure >>>>>>>>>>>> out >>>>>>>>>>>>>> whether such a new feature is a good fit for Flink and giv= e >>>>> first >>>>>>>>>>>>> pointers >>>>>>>>>>>>>> for a design to implement it. >>>>>>>>>>>>>> 2. Require contributors and committers to write design >>>>>>> documents for >>>>>>>>>>>>> all >>>>>>>>>>>>>> new features and major improvements. These documents shoul= d be >>>>>>>>> attached >>>>>>>>>>>>> to >>>>>>>>>>>>>> a JIRA issue and follow a template which needs to be defin= ed. >>>>>>>>>>>>>> 3. Extend the =E2=80=9CCoding Style Guides=E2=80=9D and ad= d patterns that are >>>>>>>>>>>> commonly >>>>>>>>>>>>>> remarked in pull requests. >>>>>>>>>>>>>> 4. Restructure the current pages into three pages: a gener= al >>>>>>> guide >>>>>>>>>>>> for >>>>>>>>>>>>>> contributions and two guides for how to contribute to code= and >>>>>>>>> website >>>>>>>>>>>>> with >>>>>>>>>>>>>> all technical issues (repository, IDE setup, build system,= >>>>> etc.) >>>>>>>>>>>>>> >>>>>>>>>>>>>> Looking forward for your comments, >>>>>>>>>>>>>> Fabian >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>> >> >=20 --NlmHuTvExx3VxCRkJtOkVtl1b2cgeXeH7 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWFlCMAAoJEFCVK48prEZ4MkAP+wUKA4edRJFexVWeHvWNJYqy 8cc9rAYHuUzdCZIQHnHBzoAzJeRhPFNhtWbVSchkDd9hrSpteixfr/rkSH8EtwEs HqDMzYofilNaTeuq6AO10NS5II8SrWLOOwlaHZt/UmARmZiOvm4XwLrakM7QVm/1 dxA2lGm9vwFV8D+bN/r3+GqgGH4pz3kdloT/QIWnRWwNql7IivZIl7p71Or78G0i 2W05Qq1ePvEI5WeW9tZMxGdA9iWDXkpdQ3fnmTHD+vhegWGt2t3K1oubNAW4ovmq vvQSAu6/BxkbCQHymmbkn7b144thCZkgHMq4A6tczBSB09agbBNZ0uaZwsEWnLcE VnniHocxMtJsdN/frYd7j6CQd1d7g8a51xaJ86diRNTI/GzlFVAxwM9jCyvRtgf5 uFiHb5qId8mCtHgi4phuve5XpzdHfQIi/lPA7IkC9sRWCicd/Ti/4V7RtSUeCJQY /olKy+54SDHpEu1Gz2+VsZDlDN1dR9b1toJfnoiX8lNPkgFDAi5NL6tZWgbGdcCh 7JjVHT30TCYwFWkD/E3hvV4OLSBdXq11Lt2h6Oz26oWhfhFjZg0z8ZgBUXNgM+g1 EKlAGW/A9JakcxsGPhcLNasnH4WVyIJVAY5w1IB1nDAeii1SD2g7saEFSIn+GNf5 xExzSM8dqSYeSO8AqLQm =wz7y -----END PGP SIGNATURE----- --NlmHuTvExx3VxCRkJtOkVtl1b2cgeXeH7--