+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 >
