I never intended for all 6 RM to be involved in every commit. Just to have 6 in order to spread the load. I just want at least two of them to verify each merge.
Op wo 13 mei 2015 om 18:32 schreef sebgoa <run...@gmail.com>: > > On May 13, 2015, at 6:07 PM, David Nalley <da...@gnsa.us> wrote: > > > On Wed, May 13, 2015 at 8:36 AM, Wilder Rodrigues > > <wrodrig...@schubergphilis.com> wrote: > >> Hi guys, > >> > >> I hope that’s not too late to react on this one. > >> > >> Having 6 RMs seems a bit too much for me. For PRs containing a few > lines of code, just bug fixes or changing maven files, python, sh, etc it > might be simple and quick. However, if we get a PR with +30 commits and 10k > lines added, it gets really difficult to get the community to test/review > the PR. So, for 2 people to go over it is already taking too long to get > the code imagine, now imagine 4 or 6. > >> > >> Rohit has done an excellent job in looking into the PRs, commenting on > them and some times testing as well. But there are things that cannot > simply get him, or perhaps other guys, to test properly a PR; having time > and environment as the main reasons. > >> > >> I would say that in case we have a PR that contains: > >> > >> 1. Documentation on the Apache CS Wiki > >> 2. Unit Tests (a lot of them, minimum 70% for the code changed) > >> 3. Marvin Test Results report - test_routers, test_vpc_routers, > test_vm_life_cycle, test_account, at least. > >> > >> Should be given priority and get less RMs involved in order to speed up > our development/release processes. Unless, of course, the people would have > time to look into the PR immediately. > >> > >> What do you think? > >> > >> Cheers, > >> Wilder > >> > > > > > > I like this. > > We have to live by our tests. So enforcing good coverage, and gating > > on good results makes sense to me. > > No human can reliably eyeball all of this. > > > > --David > > I don't think we are saying anything different to this. > > Any PR should pass the Travis tests (…and there should be more tests). > Review should not allow anything that does not have unit test either. > For new features, they should come with documentation patches as well. > > bottom line, I don't think we disagree. Or maybe I missing something. > > -sebastien > > >
