Fair enough.
I still think it's too verbose but most of the feedback was positive, so I
don't want to block this.
I think it would be good to model the form with check boxes where possible.
For example
> The runtime per-record code paths (performance sensitive): *(yes / no /
don't know)*
could b
Great! :-) If Fabian is also OK with trying it out, I would ask
Stephan to open a PR for this against Flink.
On Tue, Jul 25, 2017 at 8:50 AM, Chesnay Schepler wrote:
> I'm still apprehensive about it, but don't mind trying it out. It's not like
> it can break something.
>
>
> On 24.07.2017 18:52
I'm still apprehensive about it, but don't mind trying it out. It's not
like it can break something.
On 24.07.2017 18:52, Ufuk Celebi wrote:
What's the conclusion of last weeks discussion here?
Fabian and Chesnay raised concerns about the introductory text. Are
you still concerned?
On Wed, Ju
I think a combination of techniques would be effective:
- identifying focus areas for the next release (e.g. see Robert's thread
about 1.4)
- emphasizing design discussion in advance of a PR
- assigning reviewers and a steward in a structured way
- using a label or assignment field to 'pass the bat
@Eron Review timeliness would be great to improve.
Some observation from the past year:
There were periods where some components in Flink were making slow progress
because all committers knowledgeable in those components were busy handling
pull requests that were opened against those components, b
This seems like a good step in establishing a better PR process. I believe
the process could be improved to ensure timely and targeted review by
component experts and committers.
On Mon, Jul 17, 2017 at 9:36 AM, Stephan Ewen wrote:
> Hi all!
>
> I have reflected a bit on the pull requests and o
What's the conclusion of last weeks discussion here?
Fabian and Chesnay raised concerns about the introductory text. Are
you still concerned?
On Wed, Jul 19, 2017 at 10:04 AM, Stephan Ewen wrote:
> @Chesnay:
>
> Put text into template => contributor will have to read it
> Put link to text into t
@Chesnay:
Put text into template => contributor will have to read it
Put link to text into template => most contributors will ignore the link
Yes, that is pretty much what my observation from the past is.
On Tue, Jul 18, 2017 at 11:03 PM, Chesnay Schepler
wrote:
> I'm sorry but i can't follo
On Tue, Jul 18, 2017 at 11:03 PM, Chesnay Schepler wrote:
> I'm sorry but i can't follow your logic.
I understand where you're coming from Chesnay, but I agree with
Stephan that a link is easier to ignore than a text. I think Stephan
gave good pointers to why he thinks that people don't read the
I'm sorry but i can't follow your logic.
Put text into template => contributor will definitely read it
Put link to text into template => contributor will completely ignore the
link
The advantage of the link is we don't duplicate the contribution guide
in the docs and in the template.
Furtherm
Concerning moving text to the contributors guide:
I can only say it again: I believe the contribution guide is almost dead
text. Very few people read it.
Before the current template was introduced, new contributors rarely gave
the pull request a name with Jira number. That is a good indicator abou
--发件人：Stephan
Ewen 发送时间：2017年7月18日(星期二) 22:59收件人：dev@flink.apache.org
主 题：Re: [DISCUSS] A more thorough Pull Request check list
and template
My thinking was exactly as echoed by Gordon and Ufuk:
- The yes/no sections are also for reviewers a
Thanks for leading this discussion, Stephan. I don’t disagree with anything
that has been said but am slightly concerned that the improvements in
documenting pull requests won’t translate into the git commit messages.
Acceptance of a higher standard will be swift as long as reviewers set the
ex
I like the new template but also agree with the text being too long and would
move the intro to the contributors guide with a link in the PR template.
Regarding the questions to fill out - I'd like the headings to be short and
have the affected components last so that documentation is not lost (
My thinking was exactly as echoed by Gordon and Ufuk:
- The yes/no sections are also for reviewers a reminder of what to watch
out for.
Let's face it, probably half of the committers are not aware that these
things need to be checked implicitly against every change.
A good part of the re
On Tue, Jul 18, 2017 at 10:47 AM, Fabian Hueske wrote:
> For example even if the question about changed dependencies is answered
> with "no", the reviewer still has to check that.
But having it as a required option/text in the PR descriptions helps
reviewers to actually remember to check that. I
I fully agree with Fabian.
Multiple-choice questions provide little value to the reviewer, since the
validity has to be verified in any case. While text answers have to be
validated as well,
they give some hint to the reviewer as to how it can be verified and
which steps the
contributor did to
I like the sections about purpose, change log, and verification of the
changes.
However, I think the proposed template is too much text. This is probably
the reason why the first attempt to establish a PR template failed.
I would move most of the introduction and explanations incl. examples to
the
+1, I like this a lot.
With the previous template, it doesn’t really resonate with what we should care
about, and therefore most of the time I think contributors just delete that
template and write down something on their own.
I would also like to add: “Savepoint / checkpoint binary formats” to
I really like this and vote to change our current template.
The simple yes/no/... options are a really good idea. I would add to
your email that the questions will equally help reviewers to remember
to look at these things, which is just as important.
When we merge this, we should make sure to st
Hi all!
I have reflected a bit on the pull requests and on some of the recent
changes to Flink and some of the introduced bugs / regressions that we have
fixed.
One thing that I think would have helped is to have more explicit
information about what the pull request does and how the contributor w
21 matches
Mail list logo
