flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fabian Hueske <fhue...@gmail.com>
Subject Re: Extending and improving our "How to contribute" page
Date Mon, 28 Sep 2015 16:02:57 GMT
@Chiwan, sure. Will do that. Thanks for pointing it out :-)

2015-09-28 18:00 GMT+02:00 Chiwan Park <chiwanpark@apache.org>:

> @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 <fhueske@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 <mxm@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 <fhueske@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 <chiwanpark@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.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 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 <sewen@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 <
> >>>> 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 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 <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 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 <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 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
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>
>
>
>
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message