flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Rohrmann <trohrm...@apache.org>
Subject Re: Extending and improving our "How to contribute" page
Date Thu, 15 Oct 2015 09:58:12 GMT
Thanks for leading the effort Fabian!

On Fri, Oct 9, 2015 at 10:07 AM, Maximilian Michels <mxm@apache.org> wrote:

> Very nice work, Fabian. I think we'll have to send around a reminder
> from time to time and, perhaps, evaluate the new guidelines after some
> period of time. It's great to have these documents now as a reference.
>
> On Thu, Oct 8, 2015 at 5:36 PM, Stephan Ewen <sewen@apache.org> wrote:
> > Great, thanks Fabian!
> >
> > On Thu, Oct 8, 2015 at 5:28 PM, Henry Saputra <henry.saputra@gmail.com>
> > wrote:
> >
> >> Thanks again for leading this effort, Fabian
> >>
> >> - Henry
> >>
> >> On Thursday, October 8, 2015, Fabian Hueske <fhueske@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 <fhueske@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 <fhueske@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 <fhueske@gmail.com
> >> > <javascript:;>>:
> >> > >>
> >> > >>> @Chiwan, sure. Will do that. Thanks for pointing it out :-)
> >> > >>>
> >> > >>> 2015-09-28 18:00 GMT+02:00 Chiwan Park <chiwanpark@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 <fhueske@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 <mxm@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
<
> >> fhueske@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 <
> chiwanpark@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.saputra@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 <
> >> sewen@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 <
> >> > >>>> >>>> vasilikikalavri@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 <
> >> > >>>> ktzoumas@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 <
> >> > >>>> fhueske@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
> >> > >>>> >>>>>>>>>
> >> > >>>> >>>>>>>>
> >> > >>>> >>>>>>>
> >> > >>>> >>>>
> >> > >>>> >>>>
> >> > >>>> >>>>
> >> > >>>> >>>>
> >> > >>>> >>>>
> >> > >>>> >>>>
> >> > >>>> >>
> >> > >>>>
> >> > >>>>
> >> > >>>>
> >> > >>>>
> >> > >>>>
> >> > >>>>
> >> > >>>
> >> > >>
> >> > >
> >> >
> >>
>

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