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