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 Thu, 08 Oct 2015 11:14:03 GMT
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 <fhueske@gmail.com>:

> 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 <fhueske@gmail.com>:
>
>> 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 <fhueske@gmail.com>:
>>
>>> @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