Any other thoughts? Currently this PR has 3 approvals. If no major objection, maybe we can merge it and try the process. If it doesn't work out well, we can revert and move it to contributing guide. How does that sound?
- Sijie On Thu, Jan 3, 2019 at 6:03 PM Sijie Guo <guosi...@gmail.com> wrote: > I think putting it in a CONTRIBUTING file doesn't resolve the concerns I > have raised. > One of the problems I have seen over the pull requests is that we don't > really establish any review process > between contributors and reviewers. Important things like documentation, > website updates, and backward compatibility > are usually missed by reviewers. And contributors also don't have any > guideline for checking things when they > send out pull requests. so I think we need some forms of enforcements on > forcing both reviewers and contributors to > think about a change in same standard. Only that can help Pulsar establish > a high-quality community. > > If the community agrees on a template, we can also write a simple CI job > to check if the description is written in the format of that template. > All the pull requests can only merged when descriptions are correctly > provided. Same thing we > can apply to git commit messages. > > However I don't have strong opinion on this, if most of the people think > it is better to put in the CONTRIBUTING page, > I am fine with changing that PR. > > - Sijie > > On Wed, Jan 2, 2019 at 11:03 AM Matteo Merli <matteo.me...@gmail.com> > wrote: > >> Instead of placing this content in PULL_REQUEST_TEMPLATE.md I would rather >> have it in CONTRIBUTING.md file. >> >> The reason is that currently even the minimal PULL_REQUEST_TEMPLATE is >> being left as it is and not interpreted as a "template" to >> modifiy when creating a PR. Also I'd change the current template to just >> leave "Motivation" and "Modification" sections, removing the >> "Result" part. This template was adopted from Netty PR template but the >> result section is not very useful. >> >> Having the CONTRIBUTING page, will be linked with a yellow section, when >> creating a PR on github, so it seems to me the >> proper way to use to give instructions to new contributors. >> >> -- >> Matteo Merli >> <matteo.me...@gmail.com> >> >> >> On Wed, Jan 2, 2019 at 1:11 AM Sijie Guo <guosi...@gmail.com> wrote: >> >> > If there is no objection or no more review comments, I would like to >> merge >> > and close the issue tomorrow. >> > >> > (btw, happy new year to everyone in the community!) >> > >> > - Sijie >> > >> > On Wed, Dec 26, 2018 at 9:41 AM Sijie Guo <guosi...@gmail.com> wrote: >> > >> > > FYI. I've updated the pull request to incorporate Eren's suggestions. >> > > >> > > - Sijie >> > > >> > > On Tue, Dec 25, 2018 at 5:08 PM Jia Zhai <zhaiji...@gmail.com> wrote: >> > > >> > >> +1, for Eren's and Raj's useful comments. >> > >> >> > >> On Wed, Dec 26, 2018 at 3:36 AM Eren Avsarogullari < >> > >> erenavsarogull...@gmail.com> wrote: >> > >> >> > >> > Thanks Sijie. >> > >> > >> > >> > On Tue, 25 Dec 2018 at 18:14, Sijie Guo <guosi...@gmail.com> >> wrote: >> > >> > >> > >> > > Thank y'all for your feedback. I created a PR for this PIP - >> > >> > > https://github.com/apache/pulsar/pull/3252 >> > >> > > >> > >> > > However I missed Eren's comments before. I will incorporate your >> > >> comments >> > >> > > into the PR. >> > >> > > >> > >> > > Thank you, >> > >> > > Sijie >> > >> > > >> > >> > > On Fri, Dec 21, 2018 at 2:28 PM Eren Avsarogullari < >> > >> > > erenavsarogull...@gmail.com> wrote: >> > >> > > >> > >> > > > Hi All, >> > >> > > > >> > >> > > > +1. >> > >> > > > >> > >> > > > I have also a couple of addition: >> > >> > > > >> > >> > > > 1- Issue Id can also be added to title if we have. This >> template >> > is >> > >> > also >> > >> > > > used by Apache Spark. >> > >> > > > e.g: [Issue-Id][Component] Title >> > >> > > > >> > >> > > > 2- If created PR is a following one with existing PR, it can be >> > >> useful >> > >> > to >> > >> > > > be linked/mentioned in new one for reviewers. >> > >> > > > >> > >> > > > 3- When PR is created, single commit can be preferable. Then >> > >> incoming >> > >> > > > commits can address review feedbacks. So, reviewer can track >> > recent >> > >> > > > commits. >> > >> > > > >> > >> > > > 4- Unit/Integration Test execution time check can be useful >> (For >> > >> > general >> > >> > > > test-cases, Test execution times need to be short as much as >> > >> possible >> > >> > to >> > >> > > > keep build time under control as the long-term) >> > >> > > > >> > >> > > > 5- If build is broken due to an irrelevant test, then Issue >> > creation >> > >> > (if >> > >> > > > not exist) can be useful to track and force robustness of the >> > flaky >> > >> > test. >> > >> > > > >> > >> > > > Thanks, >> > >> > > > Eren >> > >> > > > >> > >> > > > On Fri, 21 Dec 2018 at 14:47, Yuva raj <uvar...@gmail.com> >> wrote: >> > >> > > > >> > >> > > > > Agree. For System like pulsar Documentation and Stability is >> far >> > >> more >> > >> > > > > important to gain large-scale adoption. >> > >> > > > > >> > >> > > > > On Fri, 21 Dec 2018 at 11:24, Sijie Guo <guosi...@gmail.com> >> > >> wrote: >> > >> > > > > >> > >> > > > > > Hi all, >> > >> > > > > > >> > >> > > > > > With the increase of contributions, more and more features >> are >> > >> > added >> > >> > > > > pretty >> > >> > > > > > quickly. >> > >> > > > > > However, these features are either not well documented or >> > >> > introducing >> > >> > > > > > breaking changes. >> > >> > > > > > There is no process for both contributors and reviewers to >> > >> > understand >> > >> > > > the >> > >> > > > > > impact of their changes. >> > >> > > > > > >> > >> > > > > > I am proposing improve the github pull request template to >> > add a >> > >> > > > > checklist >> > >> > > > > > for contributors >> > >> > > > > > to understand what are the impacts of their changes. It can >> > also >> > >> > > > improve >> > >> > > > > > the review process. >> > >> > > > > > >> > >> > > > > > Please take a look and let me know what you think. >> > >> > > > > > >> > >> > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > >> >> > >> https://github.com/apache/pulsar/wiki/PIP-27%3A-Add-checklist-in-github-pull-request-template >> > >> > > > > > >> > >> > > > > > - Sijie >> > >> > > > > > >> > >> > > > > >> > >> > > > > >> > >> > > > > -- >> > >> > > > > *Thanks* >> > >> > > > > >> > >> > > > > *Yuvaraj L* >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > >> >> > > >> > >> >
