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