flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthias J. Sax" <mj...@apache.org>
Subject Re: Extending and improving our "How to contribute" page
Date Thu, 08 Oct 2015 11:16:28 GMT
+1

Great!

On 10/08/2015 01:14 PM, Fabian Hueske 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 <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
View raw message