@Fabian, Could you cover FLINK-2712 in your pull request? I think that it would be better than split pull request.
Regards, Chiwan Park > On Sep 28, 2015, at 4:51 PM, Fabian Hueske <fhue...@gmail.com> wrote: > > 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 <m...@apache.org>: > >> Hi Fabian, >> >> This is a very important topic. Thanks for starting the discussion. >> >> 1) JIRA discussion >> >> Absolutely. No new feature should be introduced without a discussion. >> Frankly, I see the problem that sometimes discussions only come up >> when the pull request has been opened. However, this can be overcome >> 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. The >> 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 coding >> styles also by adding new Maven Checkstyle rules? IMHO strict rules >> 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 of 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 <fhue...@gmail.com> 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 follow 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, the >> community >>> can start a discussion whether the feature is desirable for Flink 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 JIRA >> 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 about >> whether >>> a feature request is accepted or to request a design document. We can >> make >>> design documents mandatory for changes that touch the public API. >>> >>> Regarding 3): >>> >>> I agree with Vasia, that we should collect suggestions for common >> patterns >>> and also continuously update the coding guidelines. >>> @Henry, I had best practices (exception handling, tests, etc.) in 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 configuration >>> parameters, changed behavior) >>> - Main components and classes to touch >>> >>> Cheers, >>> Fabian >>> >>> [1] http://flink.apache.org/coding-guidelines.html >>> <http://flink.apache.org/coding-guidelines.html> >>> >>> 2015-09-24 10:52 GMT+02:00 Chiwan Park <chiwanp...@apache.org>: >>> >>>> Thanks Fabian for starting the discussion. >>>> >>>> +1 for overall approach. >>>> >>>> About (1), expressing that consensus must be required for new feature >> in >>>> “How to contribute” page is very nice. Some pull 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.sapu...@gmail.com> >>>> wrote: >>>>> >>>>> Thanks again, Fabian for starting the discussions. >>>>> >>>>> For (1) and (2) I think it is good idea and will help people to >>>>> 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 requires >>>>> additional reviews of the proposed design. >>>>> I like the idea of using tag in JIRA whether new features should 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 state if >>>>> possible, throw precise Exception if possible for interfaces, etc. ? >>>>> >>>>> - Henry >>>>> >>>>> >>>>> >>>>> >>>>> On Wed, Sep 23, 2015 at 9:31 AM, Stephan Ewen <se...@apache.org> >> 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 detailed. >>>>>> >>>>>> We could also make the inverse, meaning we explicitly tag certain >>>> issues as >>>>>> "requires design document". >>>>>> >>>>>> Greetings, >>>>>> Stephan >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On Wed, Sep 23, 2015 at 5:05 PM, Vasiliki Kalavri < >>>> vasilikikala...@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 contributors. >> 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 agreed on. >>>>>>> >>>>>>> That said, I think we should also make sure that we don't raise the >>>> bar too >>>>>>> high for simple contributions. >>>>>>> >>>>>>> Regarding (1) and (2), I think we should clarify what kind of >>>>>>> additions/changes require this process to be followed. e.g. do we >> need >>>> to >>>>>>> discuss additions for which JIRAs already exist? Ideas described >> in the >>>>>>> roadmaps? Adding a new algorithm to Gelly/Flink-ML? >>>>>>> >>>>>>> Regarding (3), maybe we can all suggest some examples/patterns 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 <ktzou...@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 constitutes a >>>> feature >>>>>>>> that needs a design doc, and what should be in the doc (IMO >>>>>>>> architecture/general approach, components touched, interfaces >> changed) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Sep 23, 2015 at 2:24 PM, Fabian Hueske <fhue...@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 mailing >> list. >>>> Some >>>>>>>> of >>>>>>>>> these contributions were not accepted in the end. In other cases, >>>> pull >>>>>>>>> requests had to be heavily reworked because the approach taken >> was >>>> not >>>>>>>> the >>>>>>>>> best one. These are situations which should be avoided because >> 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 “How to contribute” and “Coding guideline” >> 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 could be >>>>>>>> improved, >>>>>>>>> IMO. >>>>>>>>> >>>>>>>>> I would like to improve these pages and propose the following >>>>>>> additions: >>>>>>>>> >>>>>>>>> 1. Request contributors and committers to start discussions 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 give 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 should be >>>> attached >>>>>>>> to >>>>>>>>> a JIRA issue and follow a template which needs to be defined. >>>>>>>>> 3. Extend the “Coding Style Guides” and add patterns that are >>>>>>> commonly >>>>>>>>> remarked in pull requests. >>>>>>>>> 4. Restructure the current pages into three pages: a general >> 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 >>>>>>>>> >>>>>>>> >>>>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>
