Hi all,

Thanks for bringing this up for this discussion, because I think it's an
important aspect.

>From my perspective, a 'definition of done' serves two purposes:
1. It informs the contributor on what's expected when making a contribution
in the form of a PR
2. It instructs the committer on what to check before accepting/merging a PR

I would use a Github template primarily to deal with the first purpose. I
think that should be short and easily understandable, preferably with as
many automated checks as possible.

I would propose something like this to condense information.

1. It is following the code contribution process, including code style and
quality guide https://flink.apache.org/contributing/contribute-code.html
2. It is covered by tests and all tests have passed
3. If it has user facing changes the documentation has been updated
according to the documentation style guide

These 3 DoD can probably be broken down into multiple automation tests:

* Run a spotless check
* Run a license check
* Compile application
* Run tests
* Run E2E tests
* Build documentation
* Check if JIRA has been mentioned and exists in the PR title and commit
message
etc.

Best regards,

Martijn

On Tue, 16 Nov 2021 at 09:08, Francesco Guardiani <france...@ververica.com>
wrote:

> +1 with Ingo proposal, the goal of the template should be to help developer
> to do a self check of his/her PR quality, not to define whether something
> is done or not. It's up to the committer to check that the "definition of
> done" is fulfilled.
>
> > The Definition of Done as suggested:
>
> This checklist makes sense to me, although it seems to me we already have
> these bullet points defined here:
> https://flink.apache.org/contributing/contribute-code.html
>
> On Tue, Nov 16, 2021 at 8:16 AM Ingo Bürk <i...@ververica.com> wrote:
>
> > Hi Joe,
> >
> > thank you for starting this discussion. Having a common agreement on what
> > to expect from a PR for it to be merged is very much a worthwhile goal.
> >
> > I'm slightly worried about the addition to the PR template. We shouldn't
> > make opening PRs even more difficult (unless it adds sufficient benefit).
> >
> > There are two main benefits to have from using templates: requiring
> > information from authors to automate certain feedback, and serving as a
> > self-control checklist for contributors.
> >
> > As it stands, a large number of PRs don't fill out the template, and I
> > haven't yet seen anyone not merge a PR over that, so de-facto we are not
> > using it for the former.
> >
> > For the latter purpose of contributors having a checklist for
> themselves, I
> > think the current template is too long already and contains the wrong
> > content. Being short here is key if we want anyone to read it, and
> > personally I would cut it down significantly to a description and a
> couple
> > of checkboxes.
> >
> > This isn't exactly the scope of your proposal, but personally I wouldn't
> > like to add even more questions that need to be filled out, especially
> > since they don't actually need to be filled out. It just creates an
> > annoying burden for contributors and is ignored by those who might
> benefit
> > most from reading it anyway.
> >
> >
> > Ingo
> >
> >
> > On Mon, Nov 15, 2021, 22:36 Johannes Moser <j...@ververica.com> wrote:
> >
> > > Dear Flink Community,
> > >
> > > We as the release managers of the 1.15 release are suggesting to
> > introduce
> > > a “Definition of Done".
> > >
> > > Let me elaborate a bit on the reasons:
> > > * During the release process for 1.14 the stability of master was
> > > sometimes in a state that made contributing to Apache Flink a bad
> > > experience.
> > > * Some of the changes that have been contributed seem to be unusable by
> > > users because of defects.
> > > * Documentation is neglected which also leads to users unable to make
> use
> > > of changes. One of the reasons is, because documentation is often
> pushed
> > to
> > > a later state.
> > >
> > > With this definition of done awareness and sensibility for these aspect
> > > should be increased. Both, for the ones who are committing and for the
> > ones
> > > that are reviewing.
> > > We focus on code quality, testing and documentation. A shared
> > > understanding is created.
> > >
> > > The Definition of Done as suggested:
> > >
> > > -
> > > A PR is done and can be merged, when:
> > >
> > > 1. It is following the code contribution process
> > > 2. It is implemented according to the code style and quality guide.
> > > 3. If it has user facing changes the documentation has been updated
> > > according to the documentation style guide.
> > > 4. It is covered by tests.
> > > 5. All tests passed.
> > > -
> > >
> > > There are two PRs to illustrate the changes.
> > > https://github.com/apache/flink-web/pull/481 <
> > > https://github.com/apache/flink-web/pull/481>
> > > https://github.com/apache/flink/pull/17801 <
> > > https://github.com/apache/flink/pull/17801>
> > >
> > >
> > > It isn’t the goal to make it harder to get changes into Apache Flink.
> It
> > > is rather the opposite of making contributing and using Apache Flink a
> > > better experience.
> > > By creating awareness a push towards quality and usability should
> happen.
> > >
> > > I’m happy to hear your feedback.
> > >
> > > Best,
> > > Joe
> >
>

Reply via email to