@Ben: Fixing a subtle bug that was introduced and that users are affected by. Fixing instable builds. Minor cosmetic changes. Changes to JavaDoc. However, PRs make an excellent tool for code reviews most of the time. I see that we probably can't stress that enough.
I'd actually think that my paragraph gave more reasons to use pull requests than the current draft does. I'd be happy if we incorporated some of my suggestions. I have removed the "Whenever possible" but kept my other points about communication and follow-up discussions: "Committers should never commit anything without going through a pull request, since that would bypass test coverage and potentially cause the build to fail due to checkstyle, etc. *In addition, pull requests ensure that changes are communicated properly and potential flaws or improvements can be spotted*. Always go through the pull request, even if you won’t wait for the code review. *Even then, comments can be provided in the pull requests after it has been merged to work on follow-ups.*" Best, Max On Wed, Mar 23, 2016 at 8:19 PM, Jean-Baptiste Onofré <[email protected]> wrote: > Hi Max, > > I would keep a "stronger statement", something like: > > "Committers always provide a pull request. This pull request has to be > merged cleanly, and doesn't break the build (including test, checkstyle, > documentation). The pull request has to be reviewed and only pushed on the > upstream when the reviewer gives the LGTM keyword as comment." > > All other situations where the committer doesn't/can't provide a PR should > be approved on the dev mailing list. > > My $0.01 > > Regards > JB > > > On 03/23/2016 07:22 PM, Maximilian Michels wrote: >> >> I didn't see this paragraph before: >> >> "Committers should never commit anything without going through a pull >> request, since that would bypass test coverage and potentially cause >> the build to fail due to checkstyle, etc. Always go through the pull >> request, even if you won’t wait for the code review." >> >> How about: >> >> "Whenever possible, commits should be reviewed in a pull request. Pull >> requests ensure that changes can be communicated properly with the >> community and potential flaws or improvements can be spotted. In >> addition, pull requests ensure proper test coverage and verification >> of the build. Whenever possible, go through the pull request, even if >> you won’t wait for the code review." >> >> - Max >> >> >> On Wed, Mar 23, 2016 at 5:33 PM, Jean-Baptiste Onofré <[email protected]> >> wrote: >>> >>> +1 >>> >>> Regards >>> JB >>> >>> >>> On 03/23/2016 05:30 PM, Davor Bonaci wrote: >>>> >>>> >>>> Thanks everyone for commenting! >>>> >>>> There were no new comments in the last several days, so we'll start >>>> moving >>>> the doc over to the Beam website. >>>> >>>> Of course, there's nothing here set in stone -- please reopen the >>>> discussion about any particular point at any time in the future. >>>> >>>> On Fri, Mar 18, 2016 at 4:44 AM, Maximilian Michels <[email protected]> >>>> wrote: >>>> >>>>> Hi Frances, >>>>> >>>>> Very nice comprehensive guide. I'll leave some comments in the doc. >>>>> >>>>> Cheers, >>>>> Max >>>>> >>>>> On Fri, Mar 18, 2016 at 11:51 AM, Sandeep Deshmukh >>>>> <[email protected]> wrote: >>>>>> >>>>>> >>>>>> The document captures the process very well and has right amount of >>>>> >>>>> >>>>> details >>>>>> >>>>>> >>>>>> for newbies too. >>>>>> >>>>>> Great work!!! >>>>>> >>>>>> Regards, >>>>>> Sandeep >>>>>> >>>>>> On Fri, Mar 18, 2016 at 10:46 AM, Siva Kalagarla < >>>>> >>>>> >>>>> [email protected]> >>>>>> >>>>>> >>>>>> wrote: >>>>>> >>>>>>> Thanks Frances, This document is helpful for newbies like myself. >>>>>>> Will >>>>>>> follow these steps over this weekend. >>>>>>> >>>>>>> On Thu, Mar 17, 2016 at 2:19 PM, Frances Perry >>>>>>> <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Beamers! >>>>>>>> >>>>>>>> We've started a draft >>>>>>>> < >>>>>>>> >>>>>>> >>>>> >>>>> >>>>> https://docs.google.com/document/d/1syFyfqIsGOYDE_Hn3ZkRd8a6ylcc64Kud9YtrGHgU0E/comment >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> for the Beam contribution guide. Please take a look and provide >>>>> >>>>> >>>>> feedback. >>>>>>>> >>>>>>>> >>>>>>>> Once things settle, we'll get this moved over on to the Beam >>>>>>>> website. >>>>>>>> >>>>>>>> Frances >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> Siva Kalagarla >>>>>>> @SivaKalagarla <https://twitter.com/SivaKalagarla> >>>>>>> >>>>> >>>> >>> >>> -- >>> Jean-Baptiste Onofré >>> [email protected] >>> http://blog.nanthrax.net >>> Talend - http://www.talend.com > > > -- > Jean-Baptiste Onofré > [email protected] > http://blog.nanthrax.net > Talend - http://www.talend.com
