So if a change requires 2 LGTM before it can be committed, does that not require that a PR be created for every change in order for us to have a forum for the LGTM consensus?
I am trying to make sure we are careful about reverting back to a free-for-all commit strategy so we don't run into issues with the stability of master again. I am obviously not trying to dictate anything, but I would like us to learn from our previous mistakes and having a clear consensus on this topic is in all of our interests. *Will STEVENS* Lead Developer *CloudOps* *| *Cloud Solutions Experts 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6 w cloudops.com *|* tw @CloudOps_ On Thu, Aug 4, 2016 at 11:45 AM, John Burwell <john.burw...@shapeblue.com> wrote: > Will, > > My point is that Rajani and I’s opinions on this topic (or any other) > carry no more weight than any other committer. We have volunteered to herd > the cats to get releases out door. > > What is CI? Within our community, there are many different definitions. > I go with the industry definition — a central system of truth that builds > and tests every commit. While I would love to have such a system, it does > not currently exist across the community (accepting that Travis CI > partially implements the concept). Our agreed upon principles do not > mention CI. They only require a 2 LGTMs (at least one for code and at > least one for test). If we want to change it, we need to ensure we have > consensus on both the terms and any changes to guidelines. > > In terms of the two commits mentioned, I will look into them and determine > whether or not I have any issues. However, if you (or anyone) has an issue > with a commit, there is no need to bottleneck around the RMs. Open a > discussion on dev@ to clarify/resolve any issues/concerns. In the > unlikely event that the we cannot find consensus on issues/concerns, then > rollback the commit. > > Thanks, > -John > > > > john.burw...@shapeblue.com > www.shapeblue.com > 53 Chandos Place, Covent Garden, London VA WC2N 4HSUK > @shapeblue > > > > On Aug 4, 2016, at 11:11 AM, Will Stevens <wstev...@cloudops.com> wrote: > > > > I am not saying that we should not let other people commit. I am saying > > that we need to be clear about the process we expect them to follow. > John, > > yes, we have the principles that you linked, but are we enforcing them? > > > > For example, these two commits were made directly to master without any > > associated PR. > > - 546a3f8884398391760b76ddcf02e6bc1f30d642 > > - fd7273b446738c0ebfae84189502dbdcd18bfd42 > > > > I know they are required and they should be non-destructive, but they did > > not follow our principles and go through CI, so do we really know. Is > that > > ok? > > > > This is my point. If we let anyone commit (which is their right as you > > correctly point out), we need to start enforcing our commit principles. > I > > think it is important to have clarity on this point ASAP so everyone is > > comfortable with these details. > > > > Cheers, > > > > *Will STEVENS* > > Lead Developer > > > > *CloudOps* *| *Cloud Solutions Experts > > 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6 > > w cloudops.com *|* tw @CloudOps_ > > > > On Thu, Aug 4, 2016 at 10:58 AM, John Burwell < > john.burw...@shapeblue.com> > > wrote: > > > >> Rajani and Will, > >> > >> It is actually not up to the release managers to make such a > >> determination. Our bylaws state that anyone with a commit bit can > commit > >> (or, if necessary, rollback a commit). By granting someone a commit > bit, > >> we have imparted a trust that an individual will protect the integrity > of > >> the codebase and work in the best interest of the community. If someone > >> makes a mistake (which has happened), we can easily rollback a commit. > We > >> have even decided to rollback commits after getting 2 LGTMs on a PR. > Under > >> the Apache Way, committers are encouraged to propel a project forward. > >> Restricting merges to RMs not only restricts velocity and it also limits > >> the energy/contributions of our large committer base. > >> > >> In terms of the rules below, we have, by consensus, accepted a set of > >> release principles [1] that specify how PRs should be reviewed and > accepted > >> for merge to master. If the guidelines outlined in that document are > not > >> followed, we have accepted that another committer may rollback a commit. > >> Rajani and I will be watching the release branches. If we find commits > >> that do not conform to our accepted merge rules, we will roll them back > and > >> work with the committer to fix the issues. > >> > >> Thanks, > >> -John > >> > >> [1]: https://cwiki.apache.org/confluence/display/CLOUDSTACK/ > >> Release+principles+for+Apache+CloudStack+4.6+and+up > >> > >>> > >> john.burw...@shapeblue.com > >> www.shapeblue.com > >> 53 Chandos Place, Covent Garden, London VA WC2N 4HSUK > >> @shapeblue > >> > >> > >> > >> On Aug 4, 2016, at 10:17 AM, Will Stevens <wstev...@cloudops.com> > wrote: > >>> > >>> I will let the RMs for this release weigh in on this, but here are my > >>> thoughts. > >>> > >>> If we let anyone commit, I think the following rules MUST be followed: > >>> - No commits directly to the repo, which are not a merge of a GitHub > Pull > >>> Request. So every change to the repo should be through `git pr ####` > >> using > >>> this tool [1]. This ensures everything that gets committed goes > through > >>> our CI pipeline and is verified before commit. It also makes it easier > >> for > >>> us to be able to script the generation of release notes and correlate > the > >>> commit history with other sources (GitHub, Jira, etc). I will submit a > >> PR > >>> with tools for generating release notes based on merged GitHub PRs > >> soon... > >>> - Every PR merged into a previous branch must be forward merged to > later > >>> branches. This is done using this tool [2] in order to make sure the > >>> commit hashes are consistent across all branches. This is for > >> auditability > >>> and comparing what exists in one branch vs another. > >>> > >>> [1] https://github.com/apache/cloudstack/blob/master/tools/git/git-pr > >>> [2] https://github.com/apache/cloudstack/blob/master/tools/ > >> git/git-fwd-merge > >>> > >>> This is my two cents anyway... > >>> > >>> *Will STEVENS* > >>> Lead Developer > >>> > >>> *CloudOps* *| *Cloud Solutions Experts > >>> 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6 > >>> w cloudops.com *|* tw @CloudOps_ > >>> > >>> On Thu, Aug 4, 2016 at 3:43 AM, Rohit Yadav <rohit.ya...@shapeblue.com > > > >>> wrote: > >>> > >>>> I disagree with having only RMs to merge PRs when we're not in freeze. > >> In > >>>> general we've implicitly honoured this behaviour but it was never > voted. > >>>> Our RMs may not be as active as we want them to be, while they are > >>>> historically good at writing policies but it's hard to put them in > >> practice > >>>> and further it's understandable that they may not be able to volunteer > >>>> enough time and effort to get the PRs sorted. > >>>> > >>>> > >>>> Over past months this and similar practices have killed our commit and > >>>> development momentum, and I think it's not a healthy practice for our > >>>> community to engage in further. Instead, we can have committers (and > in > >>>> future maybe bots) to merge a PR if they have 2 LGTMs, no objections > and > >>>> test results from both Travis (simulator) and Bubble/BVT/Trillian > (tests > >>>> against at least one and ideally all three hypervisors - KVM, Xen and > >>>> VMware). > >>>> > >>>> > >>>> Regards. > >>>> > >>>> ________________________________ > >>>> From: Rajani Karuturi <raj...@apache.org> > >>>> Sent: 03 August 2016 13:43:54 > >>>> To: dev@cloudstack.apache.org > >>>> Subject: Re: 4.10.0 release > >>>> > >>>> ouch.. looks like my email client stripped all the new lines. > >>>> Re-sending from webmail > >>>> > >>>> Hi All, > >>>> These are the proposed dates for 4.10 release (copied from another > >> thread > >>>> by John Burwell) > >>>> * Development (master open to features and defect fixes): 1 August > 2016 > >> - > >>>> 11 September 2016 > >>>> * Testing: 12 - 18 September 2016 > >>>> * RC Voting: 19 - 25 September 2016 > >>>> * Release: 26 September 2016 > >>>> > >>>> master is open for 4.10.0. > >>>> It still means that only PRs will be merged and they will be merged > >> only by > >>>> RMs ( For 4.10.0, its John Burwell and Rajani Karuturi) > >>>> Every PR should have a JIRA bug ID, 1 code review and 1 test review. > >>>> It would help in reviewing if the contributor could put information > >> about > >>>> the feature/bug and how its tested. > >>>> Also, please rebase any pending PRs you have to the latest master or > the > >>>> 4.9 release branch. > >>>> > >>>> Finally, anyone in the community can review and test PRs. We currently > >> have > >>>> huge backlog. We need everyones help in getting them merged(especially > >>>> running the tests) > >>>> Looking forward for your help in merging PRs. > >>>> Happy PR bashing!! > >>>> > >>>> Thanks, > >>>> > >>>> > >>>> > >>>> ~Rajani > >>>> > >>>> > >>>> rohit.ya...@shapeblue.com > >>>> www.shapeblue.com > >>>> 53 Chandos Place, Covent Garden, London WC2N 4HSUK > >>>> @shapeblue > >>>> > >>>> > >>>> > >>>> On Wed, Aug 3, 2016 at 1:19 PM, Erik Weber <terbol...@gmail.com> > wrote: > >>>> > >>>>> A newline or two wouldn't hurt, this is pretty hard to read tbh. > >>>>> > >>>>> -- > >>>>> Erik > >>>>> > >>>>> On Wed, Aug 3, 2016 at 9:27 AM, Rajani Karuturi <raj...@apache.org> > >>>> wrote: > >>>>> > >>>>>> Hi All,These are the proposed dates for 4.10 release (copied from > >>>>>> another thread by John Burwell)* Development (master open to > >>>>>> features and defect fixes): 1 August 2016 - 11 September 2016* > >>>>>> Testing: 12 - 18 September 2016* RC Voting: 19 - 25 September > >>>>>> 2016* Release: 26 September 2016 > >>>>>> master is open for 4.10.0. It still means that only PRs will be > >>>>>> merged and they will be merged only by RMs ( For 4.10.0, its John > >>>>>> Burwell and Rajani Karuturi)Every PR should have a JIRA bug ID, 1 > >>>>>> code review and 1 test review.It would help in reviewing if the > >>>>>> contributor could put information about the feature/bug and how > >>>>>> its tested.Also, please rebase any pending PRs you have to the > >>>>>> latest master or the 4.9 release branch. > >>>>>> Finally, anyone in the community can review and test PRs. We > >>>>>> currently have huge backlog. We need everyones help in getting > >>>>>> them merged(especially running the tests)Looking forward for your > >>>>>> help in merging PRs. Happy PR bashing!! > >>>>>> Thanks,~ Rajanihttp://cloudplatform.accelerite.com/ > >>>>> > >>>> > >> > >> > >
