Thanks again for leading this effort, Fabian - Henry
On Thursday, October 8, 2015, Fabian Hueske <fhue...@gmail.com> wrote: > Hi everybody, > > I merged our new contribution guidelines a few minutes ago. > > I'd like to emphasize that these rules do not have any effect, if nobody > follows them. > So please follow our contribution rules and make others aware of them as > well. > > Specifically > - pay attention that all PRs are backed by a JIRA and ask to create a JIRA > 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. > > Thank you, > Fabian > > 2015-10-07 10:16 GMT+02:00 Fabian Hueske <fhue...@gmail.com <javascript:;> > >: > > > Thanks for the feedback everybody. > > I updated the PR and would like to merge it later today if there are no > > more comments. > > > > Cheers, Fabian > > > > > > 2015-10-05 14:09 GMT+02:00 Fabian Hueske <fhue...@gmail.com > <javascript:;>>: > > > >> 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 <fhue...@gmail.com > <javascript:;>>: > >> > >>> @Chiwan, sure. Will do that. Thanks for pointing it out :-) > >>> > >>> 2015-09-28 18:00 GMT+02:00 Chiwan Park <chiwanp...@apache.org > <javascript:;>>: > >>> > >>>> @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 > <javascript:;>> 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 > <javascript:;>>: > >>>> > > >>>> >> 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 > <javascript:;>> > >>>> 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 > <javascript:;>>: > >>>> >>> > >>>> >>>> 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 <javascript:;>> > >>>> >>>> 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 > <javascript:;>> > >>>> >> 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 <javascript:;> > >>>> >>>>>>> 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 <javascript:;> > >>>> >>> > >>>> >>>> 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 <javascript:;> > >>>> >>> > >>>> >>>>>>> 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 > >>>> >>>>>>>>> > >>>> >>>>>>>> > >>>> >>>>>>> > >>>> >>>> > >>>> >>>> > >>>> >>>> > >>>> >>>> > >>>> >>>> > >>>> >>>> > >>>> >> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>> > >> > > >
