From my side, I also like the new template which contains more useful information, and both the reviewer and contributor may get benefits from it. For my previous pull requests as a contributor, I always listed the purpose of change and change log, but rarely mentioned how to verify the change and affected components.These are really necessary and shoule not be ignored in the pull request. So the template can indeed help contributor think more when submitting pull request. If I review other pull requests, I also want to see these sections before code review. zhijiang------------------------------------------------------------------发件人：Stephan Ewen <se...@apache.org>发送时间：2017年7月18日(星期二) 22:59收件人：dev@flink.apache.org <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 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 recent issues came from exactly that. Changes get merged (because the pull request lingered or the number of open PRs is high) and these implications are not thought through. - This is to me a tradeoff between requiring explicit +1s from certain people (maintainers) for certain components, and getting an awareness into everybody's mind. - It also makes all users aware that these things are considered and implicitly manages expectations in how fast can things get merged. Concerning the long text: I think it is fine to play the ball a bit more to the contributors. Making it easy, yes. But also making it correct and well. We need to make contributors aware of what it means to contribute to a system to runs highly available critical infrastructure. There is quite often still the mindset of "hey, cool, open source, let me throw something out there". My take is that anyone who is serious about contributing and serious about quality is not put off by this template. Concerning the introductory text: I bet that rarely anyone reads the "how to contribute" guide. Before the template, virtually no new pull request had even the required naming. That text needs to be in the template, or we might as well not have it anywhere at all. Just for reference: Below is the introductory text of the JDK ;-) 5. Know what to expect Only the best patches submitted will actually make it all the way into a JDK code base. The goal is not to take in the maximum number of contributions possible, but rather to accept only the highest-quality contributions. The JDK is used daily by millions of people and thousands of businesses, often in mission-critical applications, and so we can't afford to accept anything less than the very best. If you're relatively new to the Java platform then we recommend that you gain more experience writing Java applications before you attempt to work on the JDK itself. The purpose of the sponsored-contribution process is to bring developers who already have the skills required to work on the JDK into the existing development community. The members of that community have neither the time nor the patience required to teach basic Java programming skills or platform implementation techniques. On Tue, Jul 18, 2017 at 12:15 PM, Ufuk Celebi <u...@apache.org> wrote: > On Tue, Jul 18, 2017 at 10:47 AM, Fabian Hueske <fhue...@gmail.com> 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 think we should be > more realistic here and assume that reviewers will also overlook > things etc. > > To me, keeping the questions is more important than the intro text. > Therefore, I would be OK with moving the text to the contrib guide, > but I would definitely keep the detailed yes/nos and not go with high > level questions that everyone will answer differently. > > – Ufuk >
