You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Sebastien Goasguen <ru...@gmail.com> on 2015/06/25 16:38:55 UTC
[PROPOSAL] Commit to master through PR only
Folks,
A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
However to get started on the right track. I would like to propose the following easy step:
Starting Monday June 29th (next monday):
- Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
- Direct commit will be reverted
- Any committer can merge the PR.
Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
There is no tooling involved, just human agreement.
cheers,
-Sebastien
Re: [PROPOSAL] Commit to master through PR only
Posted by Funs Kessen <fu...@barred.org>.
Hi Seb,
Love the idea, let’s do it!
Cheers,
Funs
> On 26 Jun 2015, at 10:14, Erik Weber <te...@gmail.com> wrote:
>
> On Thu, Jun 25, 2015 at 4:38 PM, Sebastien Goasguen <ru...@gmail.com>
> wrote:
>
>> Folks,
>>
>> A few of us are in Amsterdam at DevOps days. We are chatting about release
>> management procedure.
>> Remi is working on a set of principles that he will put on the wiki to
>> start a [DISCUSS].
>>
>> However to get started on the right track. I would like to propose the
>> following easy step:
>>
>> Starting Monday June 29th (next monday):
>>
>> - Only commit through PR will land on master (after a minimum of 2 LGTM
>> and green Travis results)
>> - Direct commit will be reverted
>> - Any committer can merge the PR.
>>
>> Goal being to start having a new practice -everything through PR for
>> everyone- which is an easy way to gate our own commits building up to a PR.
>>
>> There is no tooling involved, just human agreement.
>>
>>
> If I were to add anything, it would be that all bug fixes should reference
> a Jira issue, and that new features / major improvements should have a FS
> (+ Jira ticket).
>
> Trivial fixes (typos etc), docs, regarding travis/ci etc. is fine without
> Jira tickets imho.
>
> --
> Erik
—
=Funs
Re: [PROPOSAL] Commit to master through PR only
Posted by Erik Weber <te...@gmail.com>.
On Thu, Jun 25, 2015 at 4:38 PM, Sebastien Goasguen <ru...@gmail.com>
wrote:
> Folks,
>
> A few of us are in Amsterdam at DevOps days. We are chatting about release
> management procedure.
> Remi is working on a set of principles that he will put on the wiki to
> start a [DISCUSS].
>
> However to get started on the right track. I would like to propose the
> following easy step:
>
> Starting Monday June 29th (next monday):
>
> - Only commit through PR will land on master (after a minimum of 2 LGTM
> and green Travis results)
> - Direct commit will be reverted
> - Any committer can merge the PR.
>
> Goal being to start having a new practice -everything through PR for
> everyone- which is an easy way to gate our own commits building up to a PR.
>
> There is no tooling involved, just human agreement.
>
>
If I were to add anything, it would be that all bug fixes should reference
a Jira issue, and that new features / major improvements should have a FS
(+ Jira ticket).
Trivial fixes (typos etc), docs, regarding travis/ci etc. is fine without
Jira tickets imho.
--
Erik
Re: [PROPOSAL] Commit to master through PR only
Posted by David Nalley <da...@gnsa.us>.
On Wed, Jul 1, 2015 at 3:39 AM, sebgoa <ru...@gmail.com> wrote:
>
> On Jul 1, 2015, at 9:35 AM, Wilder Rodrigues <WR...@schubergphilis.com> wrote:
>
>> Nice!
>>
>> I spent couple of hours this morning to review a few PRs.
>>
>> But we still have too many of them and not many people reviewing/testing, which makes the process a bit slow.
>>
>
> I expect this week to get slow. It's July 4th week end in the US.
>
> And it's a new process.
>
> IMHO there is no problem in being a little slow and having a back log of PRs as long as it make releasing faster.
>
> And it will motivate everyone to review.
>
Having seen this work in a few other communities, it will create a
currency out of reviews. (and that's a good thing)
You'll review someones PR in exchange for their review. People will
come to the list asking for their review to get some attention.
We'll end up having a weekly(?) PR-athon, etc.
--David
Re: [PROPOSAL] Commit to master through PR only
Posted by sebgoa <ru...@gmail.com>.
On Jul 1, 2015, at 9:35 AM, Wilder Rodrigues <WR...@schubergphilis.com> wrote:
> Nice!
>
> I spent couple of hours this morning to review a few PRs.
>
> But we still have too many of them and not many people reviewing/testing, which makes the process a bit slow.
>
I expect this week to get slow. It's July 4th week end in the US.
And it's a new process.
IMHO there is no problem in being a little slow and having a back log of PRs as long as it make releasing faster.
And it will motivate everyone to review.
> From the guys who usually review PRs, who is currently on holidays?
>
> Cheers,
> Wilder
>
>
>> On 29 Jun 2015, at 11:27, sebgoa <ru...@gmail.com> wrote:
>>
>> Ok we are on,
>>
>> Starting today, commit to master through PR only.
>> 2 LGTM needed for merge.
>> If Travis fails, we can still merge given a good explanation of why (since travis has issues once in a while).
>>
>> I will keep an eye on commit, at least once a day, and ping the list if I see a commit that went in without a PR.
>>
>> thanks, let's give this a shot, goal being of course to stabilize master for 4.6.
>>
>> Everyone should start testing master as if it were a release branch now.
>>
>> -sebastien
>>
>>
>> On Jun 28, 2015, at 9:17 AM, Remi Bergsma <re...@remi.nl> wrote:
>>
>>> Let’s do it!
>>>
>>> Starting tomorrow we’ll commit to master through PR only (as described below), and we’ll evaluate this at Sept 30, 2015.
>>>
>>> I’ll put a reminder in my schedule to start the thread.
>>>
>>> Regards,
>>> Remi
>>>
>>>> On 26 jun. 2015, at 23:10, Daan Hoogland <da...@gmail.com> wrote:
>>>>
>>>> date := 2015-09-30 ???
>>>>
>>>> On Fri, Jun 26, 2015 at 9:54 PM, David Nalley <da...@gnsa.us> wrote:
>>>>> On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>>> Folks,
>>>>>>
>>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>>>
>>>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>>>
>>>>>> Starting Monday June 29th (next monday):
>>>>>>
>>>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>>>> - Direct commit will be reverted
>>>>>> - Any committer can merge the PR.
>>>>>>
>>>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>>>
>>>>>> There is no tooling involved, just human agreement.
>>>>>>
>>>>>> cheers,
>>>>>>
>>>>>> -Sebastien
>>>>>
>>>>> In general, +1
>>>>> I think we should set a time, say a month or two out, to review how
>>>>> well it has worked, and what we need to tweak to make things better. I
>>>>> think we should be explicit with this so that we can say 'On $date'
>>>>> we'll start a thread to talk about what has and hasn't worked and how
>>>>> we can improve this.
>>>>>
>>>>> --David
>>>>
>>>>
>>>>
>>>> --
>>>> Daan
>>>
>>
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Wilder Rodrigues <WR...@schubergphilis.com>.
Nice!
I spent couple of hours this morning to review a few PRs.
But we still have too many of them and not many people reviewing/testing, which makes the process a bit slow.
>From the guys who usually review PRs, who is currently on holidays?
Cheers,
Wilder
> On 29 Jun 2015, at 11:27, sebgoa <ru...@gmail.com> wrote:
>
> Ok we are on,
>
> Starting today, commit to master through PR only.
> 2 LGTM needed for merge.
> If Travis fails, we can still merge given a good explanation of why (since travis has issues once in a while).
>
> I will keep an eye on commit, at least once a day, and ping the list if I see a commit that went in without a PR.
>
> thanks, let's give this a shot, goal being of course to stabilize master for 4.6.
>
> Everyone should start testing master as if it were a release branch now.
>
> -sebastien
>
>
> On Jun 28, 2015, at 9:17 AM, Remi Bergsma <re...@remi.nl> wrote:
>
>> Let’s do it!
>>
>> Starting tomorrow we’ll commit to master through PR only (as described below), and we’ll evaluate this at Sept 30, 2015.
>>
>> I’ll put a reminder in my schedule to start the thread.
>>
>> Regards,
>> Remi
>>
>>> On 26 jun. 2015, at 23:10, Daan Hoogland <da...@gmail.com> wrote:
>>>
>>> date := 2015-09-30 ???
>>>
>>> On Fri, Jun 26, 2015 at 9:54 PM, David Nalley <da...@gnsa.us> wrote:
>>>> On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>> Folks,
>>>>>
>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>>
>>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>>
>>>>> Starting Monday June 29th (next monday):
>>>>>
>>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>>> - Direct commit will be reverted
>>>>> - Any committer can merge the PR.
>>>>>
>>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>>
>>>>> There is no tooling involved, just human agreement.
>>>>>
>>>>> cheers,
>>>>>
>>>>> -Sebastien
>>>>
>>>> In general, +1
>>>> I think we should set a time, say a month or two out, to review how
>>>> well it has worked, and what we need to tweak to make things better. I
>>>> think we should be explicit with this so that we can say 'On $date'
>>>> we'll start a thread to talk about what has and hasn't worked and how
>>>> we can improve this.
>>>>
>>>> --David
>>>
>>>
>>>
>>> --
>>> Daan
>>
>
Re: [PROPOSAL] Commit to master through PR only
Posted by sebgoa <ru...@gmail.com>.
Ok we are on,
Starting today, commit to master through PR only.
2 LGTM needed for merge.
If Travis fails, we can still merge given a good explanation of why (since travis has issues once in a while).
I will keep an eye on commit, at least once a day, and ping the list if I see a commit that went in without a PR.
thanks, let's give this a shot, goal being of course to stabilize master for 4.6.
Everyone should start testing master as if it were a release branch now.
-sebastien
On Jun 28, 2015, at 9:17 AM, Remi Bergsma <re...@remi.nl> wrote:
> Let’s do it!
>
> Starting tomorrow we’ll commit to master through PR only (as described below), and we’ll evaluate this at Sept 30, 2015.
>
> I’ll put a reminder in my schedule to start the thread.
>
> Regards,
> Remi
>
>> On 26 jun. 2015, at 23:10, Daan Hoogland <da...@gmail.com> wrote:
>>
>> date := 2015-09-30 ???
>>
>> On Fri, Jun 26, 2015 at 9:54 PM, David Nalley <da...@gnsa.us> wrote:
>>> On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>> Folks,
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>>
>>>> -Sebastien
>>>
>>> In general, +1
>>> I think we should set a time, say a month or two out, to review how
>>> well it has worked, and what we need to tweak to make things better. I
>>> think we should be explicit with this so that we can say 'On $date'
>>> we'll start a thread to talk about what has and hasn't worked and how
>>> we can improve this.
>>>
>>> --David
>>
>>
>>
>> --
>> Daan
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Remi Bergsma <re...@remi.nl>.
Let’s do it!
Starting tomorrow we’ll commit to master through PR only (as described below), and we’ll evaluate this at Sept 30, 2015.
I’ll put a reminder in my schedule to start the thread.
Regards,
Remi
> On 26 jun. 2015, at 23:10, Daan Hoogland <da...@gmail.com> wrote:
>
> date := 2015-09-30 ???
>
> On Fri, Jun 26, 2015 at 9:54 PM, David Nalley <da...@gnsa.us> wrote:
>> On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
>>> Folks,
>>>
>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>
>>> However to get started on the right track. I would like to propose the following easy step:
>>>
>>> Starting Monday June 29th (next monday):
>>>
>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>> - Direct commit will be reverted
>>> - Any committer can merge the PR.
>>>
>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>
>>> There is no tooling involved, just human agreement.
>>>
>>> cheers,
>>>
>>> -Sebastien
>>
>> In general, +1
>> I think we should set a time, say a month or two out, to review how
>> well it has worked, and what we need to tweak to make things better. I
>> think we should be explicit with this so that we can say 'On $date'
>> we'll start a thread to talk about what has and hasn't worked and how
>> we can improve this.
>>
>> --David
>
>
>
> --
> Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
date := 2015-09-30 ???
On Fri, Jun 26, 2015 at 9:54 PM, David Nalley <da...@gnsa.us> wrote:
> On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
>> Folks,
>>
>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>
>> However to get started on the right track. I would like to propose the following easy step:
>>
>> Starting Monday June 29th (next monday):
>>
>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>> - Direct commit will be reverted
>> - Any committer can merge the PR.
>>
>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>
>> There is no tooling involved, just human agreement.
>>
>> cheers,
>>
>> -Sebastien
>
> In general, +1
> I think we should set a time, say a month or two out, to review how
> well it has worked, and what we need to tweak to make things better. I
> think we should be explicit with this so that we can say 'On $date'
> we'll start a thread to talk about what has and hasn't worked and how
> we can improve this.
>
> --David
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by David Nalley <da...@gnsa.us>.
On Thu, Jun 25, 2015 at 10:38 AM, Sebastien Goasguen <ru...@gmail.com> wrote:
> Folks,
>
> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>
> However to get started on the right track. I would like to propose the following easy step:
>
> Starting Monday June 29th (next monday):
>
> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
> - Direct commit will be reverted
> - Any committer can merge the PR.
>
> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>
> There is no tooling involved, just human agreement.
>
> cheers,
>
> -Sebastien
In general, +1
I think we should set a time, say a month or two out, to review how
well it has worked, and what we need to tweak to make things better. I
think we should be explicit with this so that we can say 'On $date'
we'll start a thread to talk about what has and hasn't worked and how
we can improve this.
--David
Re: [PROPOSAL] Commit to master through PR only
Posted by Rafael Fonseca <rs...@gmail.com>.
That's what the _Extended suffix i added means :)
On Fri, Jun 26, 2015 at 3:15 PM, Daan Hoogland <da...@gmail.com>
wrote:
> On Fri, Jun 26, 2015 at 3:13 PM, Rafael Fonseca <rs...@gmail.com>
> wrote:
> > Or if we really want the extra overhead:
> >
> > ( Green_Travis && 2LGTM) || ( Red_Travis_false_positive &&
> 3LGTM_Extended)
>
>
> or
>
> ( Green_Travis && 2LGTM) || ( Red_Travis_false_positive &&
> 2LGTM_Extended && justification in writing)
>
> that was the idea expressed
> --
> Daan
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
On Fri, Jun 26, 2015 at 3:13 PM, Rafael Fonseca <rs...@gmail.com> wrote:
> Or if we really want the extra overhead:
>
> ( Green_Travis && 2LGTM) || ( Red_Travis_false_positive && 3LGTM_Extended)
or
( Green_Travis && 2LGTM) || ( Red_Travis_false_positive &&
2LGTM_Extended && justification in writing)
that was the idea expressed
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rafael Fonseca <rs...@gmail.com>.
I did not mean to imply that you were saying red travis was fine :)
Just that it was requiring same number of people to look at it as the green
travis, of course no one should put in a LGTM on a failed travis without
looking at what the travis output was :)
Even the fuzzy stuff can be booleanized, perhaps i did over-simplify it
hehe.. full function would be:
( ( Green_Travis && 2LGTM) || ( Red_Travis_false_positive &&
2LGTM_Extended) && (!Red_Travis_real_problem_in_code) )
Just skipped the obvious last part for readability.
My point is that Green travis should also count as a vote, hence:
( Green_Travis && 1LGTM) || ( Red_Travis_false_positive && 2LGTM_Extended)
Or if we really want the extra overhead:
( Green_Travis && 2LGTM) || ( Red_Travis_false_positive && 3LGTM_Extended)
On Fri, Jun 26, 2015 at 2:46 PM, Daan Hoogland <da...@gmail.com>
wrote:
> I said that red travis is requiring extra explanation by the LGTMers
> to justify overrinding travis as an alternative to green travis. Not
> that red travis is fine.
>
> your logic is too boolean, not fuzzy enough
>
> On Fri, Jun 26, 2015 at 2:42 PM, Rafael Fonseca <rs...@gmail.com>
> wrote:
> > I agree with Daan also, but there's a conflict here..
> >
> > Initial suggestion:
> >
> > ( Green_Travis && 2LGTM)
> >
> > Daan suggested:
> >
> > ( Red_Travis && 2LGTM)
> >
> > Which would make for:
> >
> > ( Green_Travis && 2LGTM) || ( Red_Travis && 2LGTM)
> >
> > Or apply boolean logic to remove redundant parameters:
> >
> > (2LGTM)
> >
> > This would completely remove whatever travis says from the equation... if
> > we do have some trust on when travis says go... it should be:
> >
> > ( Green_Travis && 1LGTM) || ( Red_Travis && 2LGTM)
> >
> > Or if we really want the extra overhead:
> >
> > ( Green_Travis && 2LGTM) || ( Red_Travis && 3LGTM)
> >
> >
> >
> > On Fri, Jun 26, 2015 at 10:10 AM, Wilder Rodrigues <
> > WRodrigues@schubergphilis.com> wrote:
> >
> >> Clean and simple, Sebastien. I like that. :)
> >>
> >> Concerning Travis, I’m with Daan and Remi: in case of a red Travis run,
> a
> >> good analysis on the results is needed before saying no.
> >>
> >> Let’s make ACS more awesome! ;)
> >>
> >> Cheers,
> >> Wilder
> >>
> >>
> >> > On 25 Jun 2015, at 22:03, Remi Bergsma <re...@remi.nl> wrote:
> >> >
> >> > Good point Daan, I like it!
> >> >
> >> > 2015-06-25 16:49 GMT+02:00 Daan Hoogland <da...@gmail.com>:
> >> >
> >> >> I still don't think travis is reliable enough to give a definite
> 'no'.
> >> >> Two LGTMs is fine and a good argument if travis is red on why this is
> >> >> not a problem for this case.
> >> >>
> >> >> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <
> rsafonseca@gmail.com>
> >> >> wrote:
> >> >>> Couldn't make it either :'(
> >> >>>
> >> >>> I think it's a very sound idea in principle, but afraid waiting for
> two
> >> >>> LGTM might slow things down even further... up to the majority vote
> i
> >> >> guess
> >> >>> it's a good principle either way :)
> >> >>>
> >> >>> On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wido@widodh.nl
> >
> >> >> wrote:
> >> >>>
> >> >>>> -----BEGIN PGP SIGNED MESSAGE-----
> >> >>>> Hash: SHA1
> >> >>>>
> >> >>>>
> >> >>>>
> >> >>>> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> >> >>>>> Folks,
> >> >>>>>
> >> >>>>> A few of us are in Amsterdam at DevOps days. We are chatting about
> >> >>>>> release management procedure. Remi is working on a set of
> >> >>>>> principles that he will put on the wiki to start a [DISCUSS].
> >> >>>>>
> >> >>>>
> >> >>>> Bummer I couldn't make it :(
> >> >>>>
> >> >>>>> However to get started on the right track. I would like to propose
> >> >>>>> the following easy step:
> >> >>>>>
> >> >>>>> Starting Monday June 29th (next monday):
> >> >>>>>
> >> >>>>> - Only commit through PR will land on master (after a minimum of 2
> >> >>>>> LGTM and green Travis results) - Direct commit will be reverted -
> >> >>>>> Any committer can merge the PR.
> >> >>>>>
> >> >>>>> Goal being to start having a new practice -everything through PR
> >> >>>>> for everyone- which is an easy way to gate our own commits
> building
> >> >>>>> up to a PR.
> >> >>>>>
> >> >>>>
> >> >>>> I'd say this is a good idea :-)
> >> >>>>
> >> >>>>> There is no tooling involved, just human agreement.
> >> >>>>>
> >> >>>>> cheers,
> >> >>>>>
> >> >>>>> -Sebastien
> >> >>>>>
> >> >>>> -----BEGIN PGP SIGNATURE-----
> >> >>>> Version: GnuPG v1
> >> >>>>
> >> >>>> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
> >> >>>> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
> >> >>>> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
> >> >>>> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
> >> >>>> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
> >> >>>> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
> >> >>>> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
> >> >>>> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
> >> >>>> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
> >> >>>> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
> >> >>>> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
> >> >>>> gbjGNJ/+Swk3z4potjpD
> >> >>>> =SkOY
> >> >>>> -----END PGP SIGNATURE-----
> >> >>>>
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Daan
> >> >>
> >>
> >>
>
>
>
> --
> Daan
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
I said that red travis is requiring extra explanation by the LGTMers
to justify overrinding travis as an alternative to green travis. Not
that red travis is fine.
your logic is too boolean, not fuzzy enough
On Fri, Jun 26, 2015 at 2:42 PM, Rafael Fonseca <rs...@gmail.com> wrote:
> I agree with Daan also, but there's a conflict here..
>
> Initial suggestion:
>
> ( Green_Travis && 2LGTM)
>
> Daan suggested:
>
> ( Red_Travis && 2LGTM)
>
> Which would make for:
>
> ( Green_Travis && 2LGTM) || ( Red_Travis && 2LGTM)
>
> Or apply boolean logic to remove redundant parameters:
>
> (2LGTM)
>
> This would completely remove whatever travis says from the equation... if
> we do have some trust on when travis says go... it should be:
>
> ( Green_Travis && 1LGTM) || ( Red_Travis && 2LGTM)
>
> Or if we really want the extra overhead:
>
> ( Green_Travis && 2LGTM) || ( Red_Travis && 3LGTM)
>
>
>
> On Fri, Jun 26, 2015 at 10:10 AM, Wilder Rodrigues <
> WRodrigues@schubergphilis.com> wrote:
>
>> Clean and simple, Sebastien. I like that. :)
>>
>> Concerning Travis, I’m with Daan and Remi: in case of a red Travis run, a
>> good analysis on the results is needed before saying no.
>>
>> Let’s make ACS more awesome! ;)
>>
>> Cheers,
>> Wilder
>>
>>
>> > On 25 Jun 2015, at 22:03, Remi Bergsma <re...@remi.nl> wrote:
>> >
>> > Good point Daan, I like it!
>> >
>> > 2015-06-25 16:49 GMT+02:00 Daan Hoogland <da...@gmail.com>:
>> >
>> >> I still don't think travis is reliable enough to give a definite 'no'.
>> >> Two LGTMs is fine and a good argument if travis is red on why this is
>> >> not a problem for this case.
>> >>
>> >> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com>
>> >> wrote:
>> >>> Couldn't make it either :'(
>> >>>
>> >>> I think it's a very sound idea in principle, but afraid waiting for two
>> >>> LGTM might slow things down even further... up to the majority vote i
>> >> guess
>> >>> it's a good principle either way :)
>> >>>
>> >>> On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl>
>> >> wrote:
>> >>>
>> >>>> -----BEGIN PGP SIGNED MESSAGE-----
>> >>>> Hash: SHA1
>> >>>>
>> >>>>
>> >>>>
>> >>>> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
>> >>>>> Folks,
>> >>>>>
>> >>>>> A few of us are in Amsterdam at DevOps days. We are chatting about
>> >>>>> release management procedure. Remi is working on a set of
>> >>>>> principles that he will put on the wiki to start a [DISCUSS].
>> >>>>>
>> >>>>
>> >>>> Bummer I couldn't make it :(
>> >>>>
>> >>>>> However to get started on the right track. I would like to propose
>> >>>>> the following easy step:
>> >>>>>
>> >>>>> Starting Monday June 29th (next monday):
>> >>>>>
>> >>>>> - Only commit through PR will land on master (after a minimum of 2
>> >>>>> LGTM and green Travis results) - Direct commit will be reverted -
>> >>>>> Any committer can merge the PR.
>> >>>>>
>> >>>>> Goal being to start having a new practice -everything through PR
>> >>>>> for everyone- which is an easy way to gate our own commits building
>> >>>>> up to a PR.
>> >>>>>
>> >>>>
>> >>>> I'd say this is a good idea :-)
>> >>>>
>> >>>>> There is no tooling involved, just human agreement.
>> >>>>>
>> >>>>> cheers,
>> >>>>>
>> >>>>> -Sebastien
>> >>>>>
>> >>>> -----BEGIN PGP SIGNATURE-----
>> >>>> Version: GnuPG v1
>> >>>>
>> >>>> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
>> >>>> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
>> >>>> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
>> >>>> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
>> >>>> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
>> >>>> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
>> >>>> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
>> >>>> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
>> >>>> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
>> >>>> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
>> >>>> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
>> >>>> gbjGNJ/+Swk3z4potjpD
>> >>>> =SkOY
>> >>>> -----END PGP SIGNATURE-----
>> >>>>
>> >>
>> >>
>> >>
>> >> --
>> >> Daan
>> >>
>>
>>
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rafael Fonseca <rs...@gmail.com>.
I agree with Daan also, but there's a conflict here..
Initial suggestion:
( Green_Travis && 2LGTM)
Daan suggested:
( Red_Travis && 2LGTM)
Which would make for:
( Green_Travis && 2LGTM) || ( Red_Travis && 2LGTM)
Or apply boolean logic to remove redundant parameters:
(2LGTM)
This would completely remove whatever travis says from the equation... if
we do have some trust on when travis says go... it should be:
( Green_Travis && 1LGTM) || ( Red_Travis && 2LGTM)
Or if we really want the extra overhead:
( Green_Travis && 2LGTM) || ( Red_Travis && 3LGTM)
On Fri, Jun 26, 2015 at 10:10 AM, Wilder Rodrigues <
WRodrigues@schubergphilis.com> wrote:
> Clean and simple, Sebastien. I like that. :)
>
> Concerning Travis, I’m with Daan and Remi: in case of a red Travis run, a
> good analysis on the results is needed before saying no.
>
> Let’s make ACS more awesome! ;)
>
> Cheers,
> Wilder
>
>
> > On 25 Jun 2015, at 22:03, Remi Bergsma <re...@remi.nl> wrote:
> >
> > Good point Daan, I like it!
> >
> > 2015-06-25 16:49 GMT+02:00 Daan Hoogland <da...@gmail.com>:
> >
> >> I still don't think travis is reliable enough to give a definite 'no'.
> >> Two LGTMs is fine and a good argument if travis is red on why this is
> >> not a problem for this case.
> >>
> >> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com>
> >> wrote:
> >>> Couldn't make it either :'(
> >>>
> >>> I think it's a very sound idea in principle, but afraid waiting for two
> >>> LGTM might slow things down even further... up to the majority vote i
> >> guess
> >>> it's a good principle either way :)
> >>>
> >>> On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl>
> >> wrote:
> >>>
> >>>> -----BEGIN PGP SIGNED MESSAGE-----
> >>>> Hash: SHA1
> >>>>
> >>>>
> >>>>
> >>>> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> >>>>> Folks,
> >>>>>
> >>>>> A few of us are in Amsterdam at DevOps days. We are chatting about
> >>>>> release management procedure. Remi is working on a set of
> >>>>> principles that he will put on the wiki to start a [DISCUSS].
> >>>>>
> >>>>
> >>>> Bummer I couldn't make it :(
> >>>>
> >>>>> However to get started on the right track. I would like to propose
> >>>>> the following easy step:
> >>>>>
> >>>>> Starting Monday June 29th (next monday):
> >>>>>
> >>>>> - Only commit through PR will land on master (after a minimum of 2
> >>>>> LGTM and green Travis results) - Direct commit will be reverted -
> >>>>> Any committer can merge the PR.
> >>>>>
> >>>>> Goal being to start having a new practice -everything through PR
> >>>>> for everyone- which is an easy way to gate our own commits building
> >>>>> up to a PR.
> >>>>>
> >>>>
> >>>> I'd say this is a good idea :-)
> >>>>
> >>>>> There is no tooling involved, just human agreement.
> >>>>>
> >>>>> cheers,
> >>>>>
> >>>>> -Sebastien
> >>>>>
> >>>> -----BEGIN PGP SIGNATURE-----
> >>>> Version: GnuPG v1
> >>>>
> >>>> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
> >>>> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
> >>>> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
> >>>> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
> >>>> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
> >>>> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
> >>>> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
> >>>> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
> >>>> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
> >>>> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
> >>>> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
> >>>> gbjGNJ/+Swk3z4potjpD
> >>>> =SkOY
> >>>> -----END PGP SIGNATURE-----
> >>>>
> >>
> >>
> >>
> >> --
> >> Daan
> >>
>
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Wilder Rodrigues <WR...@schubergphilis.com>.
Clean and simple, Sebastien. I like that. :)
Concerning Travis, I’m with Daan and Remi: in case of a red Travis run, a good analysis on the results is needed before saying no.
Let’s make ACS more awesome! ;)
Cheers,
Wilder
> On 25 Jun 2015, at 22:03, Remi Bergsma <re...@remi.nl> wrote:
>
> Good point Daan, I like it!
>
> 2015-06-25 16:49 GMT+02:00 Daan Hoogland <da...@gmail.com>:
>
>> I still don't think travis is reliable enough to give a definite 'no'.
>> Two LGTMs is fine and a good argument if travis is red on why this is
>> not a problem for this case.
>>
>> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com>
>> wrote:
>>> Couldn't make it either :'(
>>>
>>> I think it's a very sound idea in principle, but afraid waiting for two
>>> LGTM might slow things down even further... up to the majority vote i
>> guess
>>> it's a good principle either way :)
>>>
>>> On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl>
>> wrote:
>>>
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>>
>>>>
>>>> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
>>>>> Folks,
>>>>>
>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about
>>>>> release management procedure. Remi is working on a set of
>>>>> principles that he will put on the wiki to start a [DISCUSS].
>>>>>
>>>>
>>>> Bummer I couldn't make it :(
>>>>
>>>>> However to get started on the right track. I would like to propose
>>>>> the following easy step:
>>>>>
>>>>> Starting Monday June 29th (next monday):
>>>>>
>>>>> - Only commit through PR will land on master (after a minimum of 2
>>>>> LGTM and green Travis results) - Direct commit will be reverted -
>>>>> Any committer can merge the PR.
>>>>>
>>>>> Goal being to start having a new practice -everything through PR
>>>>> for everyone- which is an easy way to gate our own commits building
>>>>> up to a PR.
>>>>>
>>>>
>>>> I'd say this is a good idea :-)
>>>>
>>>>> There is no tooling involved, just human agreement.
>>>>>
>>>>> cheers,
>>>>>
>>>>> -Sebastien
>>>>>
>>>> -----BEGIN PGP SIGNATURE-----
>>>> Version: GnuPG v1
>>>>
>>>> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
>>>> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
>>>> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
>>>> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
>>>> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
>>>> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
>>>> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
>>>> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
>>>> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
>>>> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
>>>> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
>>>> gbjGNJ/+Swk3z4potjpD
>>>> =SkOY
>>>> -----END PGP SIGNATURE-----
>>>>
>>
>>
>>
>> --
>> Daan
>>
Re: [PROPOSAL] Commit to master through PR only
Posted by Remi Bergsma <re...@remi.nl>.
Good point Daan, I like it!
2015-06-25 16:49 GMT+02:00 Daan Hoogland <da...@gmail.com>:
> I still don't think travis is reliable enough to give a definite 'no'.
> Two LGTMs is fine and a good argument if travis is red on why this is
> not a problem for this case.
>
> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com>
> wrote:
> > Couldn't make it either :'(
> >
> > I think it's a very sound idea in principle, but afraid waiting for two
> > LGTM might slow things down even further... up to the majority vote i
> guess
> > it's a good principle either way :)
> >
> > On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl>
> wrote:
> >
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >>
> >>
> >> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> >> > Folks,
> >> >
> >> > A few of us are in Amsterdam at DevOps days. We are chatting about
> >> > release management procedure. Remi is working on a set of
> >> > principles that he will put on the wiki to start a [DISCUSS].
> >> >
> >>
> >> Bummer I couldn't make it :(
> >>
> >> > However to get started on the right track. I would like to propose
> >> > the following easy step:
> >> >
> >> > Starting Monday June 29th (next monday):
> >> >
> >> > - Only commit through PR will land on master (after a minimum of 2
> >> > LGTM and green Travis results) - Direct commit will be reverted -
> >> > Any committer can merge the PR.
> >> >
> >> > Goal being to start having a new practice -everything through PR
> >> > for everyone- which is an easy way to gate our own commits building
> >> > up to a PR.
> >> >
> >>
> >> I'd say this is a good idea :-)
> >>
> >> > There is no tooling involved, just human agreement.
> >> >
> >> > cheers,
> >> >
> >> > -Sebastien
> >> >
> >> -----BEGIN PGP SIGNATURE-----
> >> Version: GnuPG v1
> >>
> >> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
> >> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
> >> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
> >> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
> >> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
> >> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
> >> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
> >> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
> >> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
> >> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
> >> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
> >> gbjGNJ/+Swk3z4potjpD
> >> =SkOY
> >> -----END PGP SIGNATURE-----
> >>
>
>
>
> --
> Daan
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Rafael Fonseca <rs...@gmail.com>.
Travis is still not there but it's already pretty close hehe.. hope to make
it 100% soon :)
On Thu, Jun 25, 2015 at 4:49 PM, Daan Hoogland <da...@gmail.com>
wrote:
> I still don't think travis is reliable enough to give a definite 'no'.
> Two LGTMs is fine and a good argument if travis is red on why this is
> not a problem for this case.
>
> On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com>
> wrote:
> > Couldn't make it either :'(
> >
> > I think it's a very sound idea in principle, but afraid waiting for two
> > LGTM might slow things down even further... up to the majority vote i
> guess
> > it's a good principle either way :)
> >
> > On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl>
> wrote:
> >
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >>
> >>
> >> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> >> > Folks,
> >> >
> >> > A few of us are in Amsterdam at DevOps days. We are chatting about
> >> > release management procedure. Remi is working on a set of
> >> > principles that he will put on the wiki to start a [DISCUSS].
> >> >
> >>
> >> Bummer I couldn't make it :(
> >>
> >> > However to get started on the right track. I would like to propose
> >> > the following easy step:
> >> >
> >> > Starting Monday June 29th (next monday):
> >> >
> >> > - Only commit through PR will land on master (after a minimum of 2
> >> > LGTM and green Travis results) - Direct commit will be reverted -
> >> > Any committer can merge the PR.
> >> >
> >> > Goal being to start having a new practice -everything through PR
> >> > for everyone- which is an easy way to gate our own commits building
> >> > up to a PR.
> >> >
> >>
> >> I'd say this is a good idea :-)
> >>
> >> > There is no tooling involved, just human agreement.
> >> >
> >> > cheers,
> >> >
> >> > -Sebastien
> >> >
> >> -----BEGIN PGP SIGNATURE-----
> >> Version: GnuPG v1
> >>
> >> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
> >> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
> >> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
> >> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
> >> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
> >> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
> >> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
> >> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
> >> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
> >> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
> >> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
> >> gbjGNJ/+Swk3z4potjpD
> >> =SkOY
> >> -----END PGP SIGNATURE-----
> >>
>
>
>
> --
> Daan
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
I still don't think travis is reliable enough to give a definite 'no'.
Two LGTMs is fine and a good argument if travis is red on why this is
not a problem for this case.
On Thu, Jun 25, 2015 at 4:47 PM, Rafael Fonseca <rs...@gmail.com> wrote:
> Couldn't make it either :'(
>
> I think it's a very sound idea in principle, but afraid waiting for two
> LGTM might slow things down even further... up to the majority vote i guess
> it's a good principle either way :)
>
> On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl> wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>
>>
>> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
>> > Folks,
>> >
>> > A few of us are in Amsterdam at DevOps days. We are chatting about
>> > release management procedure. Remi is working on a set of
>> > principles that he will put on the wiki to start a [DISCUSS].
>> >
>>
>> Bummer I couldn't make it :(
>>
>> > However to get started on the right track. I would like to propose
>> > the following easy step:
>> >
>> > Starting Monday June 29th (next monday):
>> >
>> > - Only commit through PR will land on master (after a minimum of 2
>> > LGTM and green Travis results) - Direct commit will be reverted -
>> > Any committer can merge the PR.
>> >
>> > Goal being to start having a new practice -everything through PR
>> > for everyone- which is an easy way to gate our own commits building
>> > up to a PR.
>> >
>>
>> I'd say this is a good idea :-)
>>
>> > There is no tooling involved, just human agreement.
>> >
>> > cheers,
>> >
>> > -Sebastien
>> >
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1
>>
>> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
>> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
>> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
>> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
>> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
>> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
>> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
>> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
>> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
>> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
>> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
>> gbjGNJ/+Swk3z4potjpD
>> =SkOY
>> -----END PGP SIGNATURE-----
>>
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rafael Fonseca <rs...@gmail.com>.
Couldn't make it either :'(
I think it's a very sound idea in principle, but afraid waiting for two
LGTM might slow things down even further... up to the majority vote i guess
it's a good principle either way :)
On Thu, Jun 25, 2015 at 4:41 PM, Wido den Hollander <wi...@widodh.nl> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> > Folks,
> >
> > A few of us are in Amsterdam at DevOps days. We are chatting about
> > release management procedure. Remi is working on a set of
> > principles that he will put on the wiki to start a [DISCUSS].
> >
>
> Bummer I couldn't make it :(
>
> > However to get started on the right track. I would like to propose
> > the following easy step:
> >
> > Starting Monday June 29th (next monday):
> >
> > - Only commit through PR will land on master (after a minimum of 2
> > LGTM and green Travis results) - Direct commit will be reverted -
> > Any committer can merge the PR.
> >
> > Goal being to start having a new practice -everything through PR
> > for everyone- which is an easy way to gate our own commits building
> > up to a PR.
> >
>
> I'd say this is a good idea :-)
>
> > There is no tooling involved, just human agreement.
> >
> > cheers,
> >
> > -Sebastien
> >
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
>
> iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
> wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
> BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
> XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
> hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
> XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
> BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
> Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
> CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
> r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
> aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
> gbjGNJ/+Swk3z4potjpD
> =SkOY
> -----END PGP SIGNATURE-----
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Wido den Hollander <wi...@widodh.nl>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/25/2015 04:38 PM, Sebastien Goasguen wrote:
> Folks,
>
> A few of us are in Amsterdam at DevOps days. We are chatting about
> release management procedure. Remi is working on a set of
> principles that he will put on the wiki to start a [DISCUSS].
>
Bummer I couldn't make it :(
> However to get started on the right track. I would like to propose
> the following easy step:
>
> Starting Monday June 29th (next monday):
>
> - Only commit through PR will land on master (after a minimum of 2
> LGTM and green Travis results) - Direct commit will be reverted -
> Any committer can merge the PR.
>
> Goal being to start having a new practice -everything through PR
> for everyone- which is an easy way to gate our own commits building
> up to a PR.
>
I'd say this is a good idea :-)
> There is no tooling involved, just human agreement.
>
> cheers,
>
> -Sebastien
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJVjBMAAAoJEAGbWC3bPspCRYkP/jGuB3qelhlwNKY0UJZVs43T
wh3+8TKO2OTuchR4TLqJDLpWcpaHYamxukDDwNyI2+7hpZuNNnT6t4KhA5CpSITj
BVa8M9nBJAKXjPcnSPNCE8RYA6BPfwnywupwA294rnNcclDurzdHd6WssE0VCH0g
XDM8vuA1tKx55B5TTQSNwDNdlai6aaB/xTQRoFXQWEUwwkyDZF16kvYvglhycVKn
hpg/tpl4VEGCA3G5ddX3fFGDYYUFYoAYO62zpLaq9xUQN2iVny3LO9LhznfXqUc2
XUaksY9hW/8HgaeipbbbWekRZ3J/XCc9/fchFna41WlJOxju49Do5nVTtV3UdBVh
BVBW7NTmnlX3Bs9zyFyp21SIvbQMRDLTolHx0GH9rPhU34l9ww/10MEBPNP0wS7K
Xg/0TpsAviUijqKjxNbXMG+bTaPMrUtDHuoJMWUbGf+KVHVlUdNvshaURlL8SAFW
CIRWhj5Ww+rRyIrpXjC7zXv/qg7aTPD1e02nV7XfoldyDRe72QUmwa7umwZkjvQ6
r9Fxu9S0fySbakAWxYVGjQbCpK+xGCY0ndzH/eYNnf8SX2MGIEKapbJ0kkTWvTu7
aQvV/Y9hLAGMNlYCPiAK4eBFgNc7wdG/D+ZZ6t8Oxmb5O9WMlBCddvvX4mn5UlIo
gbjGNJ/+Swk3z4potjpD
=SkOY
-----END PGP SIGNATURE-----
Re: [PROPOSAL] Commit to master through PR only
Posted by Rajani Karuturi <ra...@apache.org>.
I do the same Erik. Sometimes I merge the changes from the authors branch
directly without creating a local copy (using the command mentioned in the
pull request mail).
+1 on 2 manual reviews per PR irrespective of how trivial it is.
-1 on squashed commits. If the author thinks that the change deserves more
than one commit, it should be that way. Fast forwarding >1 commits and
merging doesnt make any sense. The PR information is lost on all the
previous commits except for the one where the PR is mentioned.
~Rajani
On Thu, Jul 2, 2015 at 1:13 AM, Erik Weber <te...@gmail.com> wrote:
> On Wed, Jul 1, 2015 at 7:48 PM, Rohit Yadav <ro...@shapeblue.com>
> wrote:
>
> > Hi,
> >
> > > On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com>
> wrote:
> > >
> > > A few of us are in Amsterdam at DevOps days. We are chatting about
> > release management procedure.
> > > Remi is working on a set of principles that he will put on the wiki to
> > start a [DISCUSS].
> > >
> > > However to get started on the right track. I would like to propose the
> > following easy step:
> > >
> > > Starting Monday June 29th (next monday):
> > >
> > > - Only commit through PR will land on master (after a minimum of 2 LGTM
> > and green Travis results)
> > > - Direct commit will be reverted
> > > - Any committer can merge the PR.
> >
> > +1
> >
> > I’ve been trying to help close PRs, it was difficult at first but then I
> > found some tooling to help me do that. I think it’s certainly do-able
> > without investing a lot of effort to do it, perhaps can done everyday or
> > every few days in a week.
> >
> > Some suggestions and comments to improve PR reviewing/merging:
> >
> > - Let's merge the PR commits in a fast forward way instead of doing a
> > branch merge that introduces frivolous merge commits. This is one
> approach
> > to do quickly and painlessly:
> >
> >
> >
> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
> >
> >
>
> I'm no git expert, so I don't know if the downloading of patches and
> applying them has any benefits over branch merging, but what I usually do
> is this:
>
> (github is my remote name for the github repo)
>
> $ git fetch github pull/<PR#>/head:<local branch name>
> <review/test locally as needed>
> $ git checkout master (or whatever branch you're merging to)
> $ git merge <local branch name>
>
> --
> Erik
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Erik Weber <te...@gmail.com>.
On Wed, Jul 1, 2015 at 7:48 PM, Rohit Yadav <ro...@shapeblue.com>
wrote:
> Hi,
>
> > On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
> >
> > A few of us are in Amsterdam at DevOps days. We are chatting about
> release management procedure.
> > Remi is working on a set of principles that he will put on the wiki to
> start a [DISCUSS].
> >
> > However to get started on the right track. I would like to propose the
> following easy step:
> >
> > Starting Monday June 29th (next monday):
> >
> > - Only commit through PR will land on master (after a minimum of 2 LGTM
> and green Travis results)
> > - Direct commit will be reverted
> > - Any committer can merge the PR.
>
> +1
>
> I’ve been trying to help close PRs, it was difficult at first but then I
> found some tooling to help me do that. I think it’s certainly do-able
> without investing a lot of effort to do it, perhaps can done everyday or
> every few days in a week.
>
> Some suggestions and comments to improve PR reviewing/merging:
>
> - Let's merge the PR commits in a fast forward way instead of doing a
> branch merge that introduces frivolous merge commits. This is one approach
> to do quickly and painlessly:
>
>
> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>
>
I'm no git expert, so I don't know if the downloading of patches and
applying them has any benefits over branch merging, but what I usually do
is this:
(github is my remote name for the github repo)
$ git fetch github pull/<PR#>/head:<local branch name>
<review/test locally as needed>
$ git checkout master (or whatever branch you're merging to)
$ git merge <local branch name>
--
Erik
Re: [PROPOSAL] Commit to master through PR only
Posted by John Burwell <jo...@shapeblue.com>.
Daan,
Having worked in an environment where PRs are required for all merges, tooling is only way to ensure it is followed without creating a tremendous human burden. The tooling is not difficult to implement (and there are a number of options beside the one I suggested), and reduces (or eliminates) the need for people to go through the history to find improper commits.
Thanks,
-John
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 1, 2015, at 3:19 PM, Daan Hoogland <da...@gmail.com> wrote:
>
> On Wed, Jul 1, 2015 at 8:44 PM, John Burwell <jo...@shapeblue.com> wrote:
>> All,
>>
>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>
> agree
>
>>
>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>
> bad practice in my not so humble opinion.
>
>>
>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>
> Tooling is great but let us first find a practice to agree on and then
> fit tooling on it.
>
>>
>> Thanks,
>> -John
>>
>> [1]: https://github.com/graydon/bors
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>
>>> Hi,
>>>
>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>
>>> +1
>>>
>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>
>>> Some suggestions and comments to improve PR reviewing/merging:
>>>
>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>
>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>
>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>
>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>
>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>
>>> Regards,
>>> Rohit Yadav
>>> Software Architect, ShapeBlue
>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>
>>>
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
>
>
> --
> Daan
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
On Wed, Jul 1, 2015 at 8:44 PM, John Burwell <jo...@shapeblue.com> wrote:
> All,
>
> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
agree
>
> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
bad practice in my not so humble opinion.
>
> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
Tooling is great but let us first find a practice to agree on and then
fit tooling on it.
>
> Thanks,
> -John
>
> [1]: https://github.com/graydon/bors
>
> ---
> John Burwell (@john_burwell)
> VP of Software Engineering, ShapeBlue
> (571) 403-2411 | +44 20 3603 0542
> http://www.shapeblue.com
>
>
>
>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>
>> Hi,
>>
>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>
>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>
>>> However to get started on the right track. I would like to propose the following easy step:
>>>
>>> Starting Monday June 29th (next monday):
>>>
>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>> - Direct commit will be reverted
>>> - Any committer can merge the PR.
>>
>> +1
>>
>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>
>> Some suggestions and comments to improve PR reviewing/merging:
>>
>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>
>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>
>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>
>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>
>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>
>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>
>>> There is no tooling involved, just human agreement.
>>>
>>> cheers,
>>
>> Regards,
>> Rohit Yadav
>> Software Architect, ShapeBlue
>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>
>>
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by sebgoa <ru...@gmail.com>.
On Jul 2, 2015, at 8:43 AM, Wilder Rodrigues <WR...@schubergphilis.com> wrote:
> Sateesh and Rajesh,
>
> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>
> I wil give you the simplest example that could come to my mind to encourage no squash:
>
> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>
In our overall effort, I think squashing discussion is lower priority. Agreeing to do PR reviews with 2 LGTM before merge is already a very big step in stabilizing master and helping with releases.
that should be our higher priority.
That said, I work on libcloud as well. It is a much smaller project. But there we squash before committing on master. There can be several commits in the PR, because one will surely amend the PR based on review. But once it's accepted, it gets merged as a single commit.
You all know I am not a hard core dev guy like some of you Java gurus.
The benefit I see in squashing is that in your release branch (master), one functionality is encapsulated in a single commit. You don't need to look through 10-20 commits whose messages seem unrelated. You might be able to run some git magic to get all of it, but when I look a the basic commit history, things are right there in front of me.
Now if I want to create a bug fix release, and I choose to cherry-pick that very thing. I cherry pick one commit, and that's it.
So IMHO, the history of the commit should be in the PR review, not on the master git history.
I dream for a day with a super clean master commit history of only merges/PR, with JIRA bug ID.
-sebastien
> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>
> Would you like to see it quashed and all messed up? It would be very difficult to review.
>
> That’s just a simple example.
>
> Cheers,
> Wilder
>
>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>
>> +1 for squashing commit
>>
>> -----Original Message-----
>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>> Sent: Thursday, July 2, 2015 12:14 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>
>> All,
>>
>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>
>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>
>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>
>> Thanks,
>> -John
>>
>> [1]: https://github.com/graydon/bors
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>
>>> Hi,
>>>
>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>
>>> +1
>>>
>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>
>>> Some suggestions and comments to improve PR reviewing/merging:
>>>
>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>
>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>
>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>
>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>
>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>
>>> Regards,
>>> Rohit Yadav
>>> Software Architect, ShapeBlue
>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>
>>>
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
Wilder, you are being to friendly IMHO here. I would actually -1 a PR
that contains a commit that tries to do more then one thing. It is
obfuscating and makes it harder to discuss implementations and fixes.
<psuedo-quote>in commit abcdef, in the part that fixes the NPE related
to the network implementation of sdn-xyz in hypervisor-123, a problem
arises that had not been there if in commit fedcba for the TO of the
vm we had... </psuedo-quote>
such argumentations should be "commit abcdef wasn't necessary, just
revert commit fedcba".
On Thu, Jul 2, 2015 at 8:43 AM, Wilder Rodrigues
<WR...@schubergphilis.com> wrote:
> Sateesh and Rajesh,
>
> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>
> I wil give you the simplest example that could come to my mind to encourage no squash:
>
> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>
> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>
> Would you like to see it quashed and all messed up? It would be very difficult to review.
>
> That’s just a simple example.
>
> Cheers,
> Wilder
>
>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>
>> +1 for squashing commit
>>
>> -----Original Message-----
>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>> Sent: Thursday, July 2, 2015 12:14 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>
>> All,
>>
>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>
>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>
>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>
>> Thanks,
>> -John
>>
>> [1]: https://github.com/graydon/bors
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>
>>> Hi,
>>>
>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>
>>> +1
>>>
>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>
>>> Some suggestions and comments to improve PR reviewing/merging:
>>>
>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>
>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>
>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>
>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>
>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>
>>> Regards,
>>> Rohit Yadav
>>> Software Architect, ShapeBlue
>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>
>>>
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by John Burwell <jo...@shapeblue.com>.
Wilder,
Putting on my project manager’s hat, design documents express the intentions of a feature or enhancement before implementation begins. Typically, design documents are out of sync with the initial implementation, and they are certainly out of sync with those changes as the system evolves. In the long term, the canonical representation of a system is the code. Therefore, it is best to capture the intention of changes with the code itself as it evolves. Not only is it the most cohesive (e.g. working directly with git blame and bisect), it is the only reliable, sane way to track information as the system evolves.
Thanks,
-John
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 3, 2015, at 6:01 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>
> Hi John,
>
> If you look at the discrete operations wearing a hat of a Project Manager, you won’t care… neither would I. However, from a Software Engineer perspective, as much as the other people contributing with the Java code, I do care and it really makes reviewing the PR easier.
>
> As a consumer of my change (project manager hat again), you should be looking for Design Documents. Those will for sure show the motivation behind the changes in a higher level.
>
> Concerning the 5k lines classes, I have found a few of them and they haven been refactored accordingly. Have a look at the Virtual Router, Citrix/LibVirt resource classes, those were cleaned as much as they could. The example I gave was simple and should not be used in such a way, Think of it as a 100 lines class instead, perhaps it will help.
>
> I’m feeling inclined to send my next PR with squashed commits to see if it will get reviewed properly and in an easy way.
>
> Cheers,
> Wilder
>
>
>> On 02 Jul 2015, at 20:35, John Burwell <jo...@shapeblue.com> wrote:
>>
>> Wilder,
>>
>> In the grand scheme of the entire project history (e.g. reading git log), why do I care about these discrete operations? In six months (or long), I (as the consumer of your change) want to know what motivated this change which is completely lost in those two commits. I have found this advice [1] for commit messages combined with squashing commits to a topic (e.g. defect ticket, feature, enhancement ticket, etc) yields a git history that incredible value over the long term in a projects with a lot of developers making many changes.
>>
>> Thanks,
>> -John
>>
>> P.S. As an aside, if you encounter a 5000 class, I would encourage you to decompose it rather than reformat the file.
>>
>> [1]: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 2, 2015, at 2:43 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>>>
>>> Sateesh and Rajesh,
>>>
>>> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>>>
>>> I wil give you the simplest example that could come to my mind to encourage no squash:
>>>
>>> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
>>> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>>>
>>> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>>>
>>> Would you like to see it quashed and all messed up? It would be very difficult to review.
>>>
>>> That’s just a simple example.
>>>
>>> Cheers,
>>> Wilder
>>>
>>>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>>>
>>>> +1 for squashing commit
>>>>
>>>> -----Original Message-----
>>>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>>>> Sent: Thursday, July 2, 2015 12:14 AM
>>>> To: dev@cloudstack.apache.org
>>>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>>>
>>>> All,
>>>>
>>>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>>>
>>>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>>>
>>>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>>>
>>>> Thanks,
>>>> -John
>>>>
>>>> [1]: https://github.com/graydon/bors
>>>>
>>>> ---
>>>> John Burwell (@john_burwell)
>>>> VP of Software Engineering, ShapeBlue
>>>> (571) 403-2411 | +44 20 3603 0542
>>>> http://www.shapeblue.com
>>>>
>>>>
>>>>
>>>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>>>
>>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>>>
>>>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>>>
>>>>>> Starting Monday June 29th (next monday):
>>>>>>
>>>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>>>> - Direct commit will be reverted
>>>>>> - Any committer can merge the PR.
>>>>>
>>>>> +1
>>>>>
>>>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>>>
>>>>> Some suggestions and comments to improve PR reviewing/merging:
>>>>>
>>>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>>>
>>>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>>>
>>>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>>>
>>>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>>>
>>>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>>>
>>>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>>>
>>>>>> There is no tooling involved, just human agreement.
>>>>>>
>>>>>> cheers,
>>>>>
>>>>> Regards,
>>>>> Rohit Yadav
>>>>> Software Architect, ShapeBlue
>>>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>>>
>>>>>
>>>>>
>>>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>>>
>>>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>>>
>>>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>>>
>>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>>
>>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>>
>>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>>
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
RE: [PROPOSAL] Commit to master through PR only
Posted by Paul Angus <pa...@shapeblue.com>.
I agree with Daan,
Lazy consensus QA is not going to improve code quality. If anything we're highlighting a structural issue with the amount of resource available to review these PRs - which is a different issue and would need looking at as such as it is fundamental to our push for improved quality.
Regards
Paul Angus
VP Technology/Cloud Architect
S: +44 20 3603 0540 | M: +447711418784 | T: CloudyAngus
paul.angus@shapeblue.com
-----Original Message-----
From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
Sent: Thursday, July 9, 2015 11:25 AM
To: dev
Subject: Re: [PROPOSAL] Commit to master through PR only
On Thu, Jul 9, 2015 at 12:04 PM, Rohit Yadav <ro...@shapeblue.com>
wrote:
>
> On 09-Jul-2015, at 2:56 pm, Daan Hoogland <da...@gmail.com> wrote:
>
> I like the idea but think that 72 hours is way to short
>
>
> I think 72 hours (note: no counting weekends) should be good enough,
> which is the window for our release/vote process as well. We can
> increase this to perhaps 120 hours (5 days, except weekends).
>
7 days (including weekends even seem short to me. The objective is to make shore all code is reviewed!
> Doing this would allow at the committers to get their work merged
> without waiting on others. If the PR has one commit (or squashed into
> one commit), it would be easier to revert their merged PR if a future issue is found.
>
The idea is that committers do wait on others. Reviews are required for a reason. A merge commit can easily be reverted as well. Not related to this discussion.
> and also a
> committers must have shown effort to attract attention to their PR by
> more then the old 'mail-and-forget' management method.
>
>
> Given that committers/pmc-members are recognised as a contributors
> working in their free time, it will be challenging to enforce
> committers/pmc-members to participate in PR reviews.
>
No one is expected to work in their free time. I know a lot of us do but I would say we do most work in sponsored time.
--
Daan
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
On Thu, Jul 9, 2015 at 12:04 PM, Rohit Yadav <ro...@shapeblue.com>
wrote:
>
> On 09-Jul-2015, at 2:56 pm, Daan Hoogland <da...@gmail.com> wrote:
>
> I like the idea but think that 72 hours is way to short
>
>
> I think 72 hours (note: no counting weekends) should be good enough,
> which is the window for our release/vote process as well. We can increase
> this to perhaps 120 hours (5 days, except weekends).
>
7 days (including weekends even seem short to me. The objective is to make
shore all code is reviewed!
> Doing this would allow at the committers to get their work merged without
> waiting on others. If the PR has one commit (or squashed into one commit),
> it would be easier to revert their merged PR if a future issue is found.
>
The idea is that committers do wait on others. Reviews are required for a
reason. A merge commit can easily be reverted as well. Not related to this
discussion.
> and also a
> committers must have shown effort to attract attention to their PR by more
> then the old 'mail-and-forget' management method.
>
>
> Given that committers/pmc-members are recognised as a contributors
> working in their free time, it will be challenging to enforce
> committers/pmc-members to participate in PR reviews.
>
No one is expected to work in their free time. I know a lot of us do but I
would say we do most work in sponsored time.
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rohit Yadav <ro...@shapeblue.com>.
On 09-Jul-2015, at 2:56 pm, Daan Hoogland <da...@gmail.com>> wrote:
I like the idea but think that 72 hours is way to short
I think 72 hours (note: no counting weekends) should be good enough, which is the window for our release/vote process as well. We can increase this to perhaps 120 hours (5 days, except weekends).
Doing this would allow at the committers to get their work merged without waiting on others. If the PR has one commit (or squashed into one commit), it would be easier to revert their merged PR if a future issue is found.
and also a
committers must have shown effort to attract attention to their PR by more
then the old 'mail-and-forget' management method.
Given that committers/pmc-members are recognised as a contributors working in their free time, it will be challenging to enforce committers/pmc-members to participate in PR reviews.
Regards,
Rohit Yadav
Software Architect, ShapeBlue
[cid:9DD97B41-04C5-45F0-92A7-951F3E962F7A]
M. +91 88 262 30892 | rohit.yadav@shapeblue.com<ma...@shapeblue.com>
Blog: bhaisaab.org<http://bhaisaab.org> | Twitter: @_bhaisaab
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
On Thu, Jul 9, 2015 at 10:51 AM, Rohit Yadav <ro...@shapeblue.com>
wrote:
>
> On 09-Jul-2015, at 2:14 pm, Rohit Yadav <ro...@shapeblue.com> wrote:
>
> - This seems to be already failing, under the Apache way IMO there is no
> way we can enforce and ensure that at least two people would review any and
> every PR. There are already a growing number of open PRs that we cannot
> close unless we get 2 LGTM. Should we put some sort of maximum wait time on
> PRs that attract no attention (review or discussions), say of 72 hours
> (excluding weekends) so at least committers can merge their changes?
>
>
> Forgot to explicitly add in the suggestion above - Such attention-deficit
> PRs (no review, discussion or comment of any sorts) should only be merged
> if Travis is green along with other automated jenkins jobs (rat, build etc)
> we’ve setup.
>
I like the idea but think that 72 hours is way to short and also a
committers must have shown effort to attract attention to their PR by more
then the old 'mail-and-forget' management method.
>
>
> Regards,
> Rohit Yadav
>
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rohit Yadav <ro...@shapeblue.com>.
On 09-Jul-2015, at 2:14 pm, Rohit Yadav <ro...@shapeblue.com>> wrote:
- This seems to be already failing, under the Apache way IMO there is no way we can enforce and ensure that at least two people would review any and every PR. There are already a growing number of open PRs that we cannot close unless we get 2 LGTM. Should we put some sort of maximum wait time on PRs that attract no attention (review or discussions), say of 72 hours (excluding weekends) so at least committers can merge their changes?
Forgot to explicitly add in the suggestion above - Such attention-deficit PRs (no review, discussion or comment of any sorts) should only be merged if Travis is green along with other automated jenkins jobs (rat, build etc) we’ve setup.
Regards,
Rohit Yadav
Software Architect, ShapeBlue
[cid:9DD97B41-04C5-45F0-92A7-951F3E962F7A]
M. +91 88 262 30892 | rohit.yadav@shapeblue.com<ma...@shapeblue.com>
Blog: bhaisaab.org<http://bhaisaab.org> | Twitter: @_bhaisaab
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Rohit Yadav <ro...@shapeblue.com>.
On 07-Jul-2015, at 1:09 pm, sebgoa <ru...@gmail.com>> wrote:
The PR should not be squashed until it's reviewed and accepted.
I am only arguing for squashing it when it is accepted and before merge.
For now, I would love for us to focus on the 2 LGTM and green tests (as much as we can get them green). We can fine tune later.
Two issues;
- This seems to be already failing, under the Apache way IMO there is no way we can enforce and ensure that at least two people would review any and every PR. There are already a growing number of open PRs that we cannot close unless we get 2 LGTM. Should we put some sort of maximum wait time on PRs that attract no attention (review or discussions), say of 72 hours (excluding weekends) so at least committers can merge their changes?
- In case of a release that is going to happen and feature freeze has started, in that case will new feature work and refactoring work be on hold to be merged on master. For example, features being worked on right now, what will happen to them as master/4.6 may not allow them to be merged (as they are new features).
Regards,
Rohit Yadav
Software Architect, ShapeBlue
[cid:9DD97B41-04C5-45F0-92A7-951F3E962F7A]
M. +91 88 262 30892 | rohit.yadav@shapeblue.com<ma...@shapeblue.com>
Blog: bhaisaab.org<http://bhaisaab.org> | Twitter: @_bhaisaab
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by sebgoa <ru...@gmail.com>.
On Jul 3, 2015, at 12:01 PM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
> Hi John,
>
> If you look at the discrete operations wearing a hat of a Project Manager, you won’t care… neither would I. However, from a Software Engineer perspective, as much as the other people contributing with the Java code, I do care and it really makes reviewing the PR easier.
>
The PR should not be squashed until it's reviewed and accepted.
I am only arguing for squashing it when it is accepted and before merge.
For now, I would love for us to focus on the 2 LGTM and green tests (as much as we can get them green). We can fine tune later.
-sebastien
> As a consumer of my change (project manager hat again), you should be looking for Design Documents. Those will for sure show the motivation behind the changes in a higher level.
>
> Concerning the 5k lines classes, I have found a few of them and they haven been refactored accordingly. Have a look at the Virtual Router, Citrix/LibVirt resource classes, those were cleaned as much as they could. The example I gave was simple and should not be used in such a way, Think of it as a 100 lines class instead, perhaps it will help.
>
> I’m feeling inclined to send my next PR with squashed commits to see if it will get reviewed properly and in an easy way.
>
> Cheers,
> Wilder
>
>
>> On 02 Jul 2015, at 20:35, John Burwell <jo...@shapeblue.com> wrote:
>>
>> Wilder,
>>
>> In the grand scheme of the entire project history (e.g. reading git log), why do I care about these discrete operations? In six months (or long), I (as the consumer of your change) want to know what motivated this change which is completely lost in those two commits. I have found this advice [1] for commit messages combined with squashing commits to a topic (e.g. defect ticket, feature, enhancement ticket, etc) yields a git history that incredible value over the long term in a projects with a lot of developers making many changes.
>>
>> Thanks,
>> -John
>>
>> P.S. As an aside, if you encounter a 5000 class, I would encourage you to decompose it rather than reformat the file.
>>
>> [1]: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 2, 2015, at 2:43 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>>>
>>> Sateesh and Rajesh,
>>>
>>> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>>>
>>> I wil give you the simplest example that could come to my mind to encourage no squash:
>>>
>>> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
>>> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>>>
>>> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>>>
>>> Would you like to see it quashed and all messed up? It would be very difficult to review.
>>>
>>> That’s just a simple example.
>>>
>>> Cheers,
>>> Wilder
>>>
>>>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>>>
>>>> +1 for squashing commit
>>>>
>>>> -----Original Message-----
>>>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>>>> Sent: Thursday, July 2, 2015 12:14 AM
>>>> To: dev@cloudstack.apache.org
>>>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>>>
>>>> All,
>>>>
>>>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>>>
>>>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>>>
>>>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>>>
>>>> Thanks,
>>>> -John
>>>>
>>>> [1]: https://github.com/graydon/bors
>>>>
>>>> ---
>>>> John Burwell (@john_burwell)
>>>> VP of Software Engineering, ShapeBlue
>>>> (571) 403-2411 | +44 20 3603 0542
>>>> http://www.shapeblue.com
>>>>
>>>>
>>>>
>>>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>>>
>>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>>>
>>>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>>>
>>>>>> Starting Monday June 29th (next monday):
>>>>>>
>>>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>>>> - Direct commit will be reverted
>>>>>> - Any committer can merge the PR.
>>>>>
>>>>> +1
>>>>>
>>>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>>>
>>>>> Some suggestions and comments to improve PR reviewing/merging:
>>>>>
>>>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>>>
>>>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>>>
>>>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>>>
>>>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>>>
>>>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>>>
>>>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>>>
>>>>>> There is no tooling involved, just human agreement.
>>>>>>
>>>>>> cheers,
>>>>>
>>>>> Regards,
>>>>> Rohit Yadav
>>>>> Software Architect, ShapeBlue
>>>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>>>
>>>>>
>>>>>
>>>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>>>
>>>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>>>
>>>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>>>
>>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>>
>>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>>
>>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>>
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
Re: [PROPOSAL] Commit to master through PR only
Posted by Wilder Rodrigues <WR...@schubergphilis.com>.
Hi John,
If you look at the discrete operations wearing a hat of a Project Manager, you won’t care… neither would I. However, from a Software Engineer perspective, as much as the other people contributing with the Java code, I do care and it really makes reviewing the PR easier.
As a consumer of my change (project manager hat again), you should be looking for Design Documents. Those will for sure show the motivation behind the changes in a higher level.
Concerning the 5k lines classes, I have found a few of them and they haven been refactored accordingly. Have a look at the Virtual Router, Citrix/LibVirt resource classes, those were cleaned as much as they could. The example I gave was simple and should not be used in such a way, Think of it as a 100 lines class instead, perhaps it will help.
I’m feeling inclined to send my next PR with squashed commits to see if it will get reviewed properly and in an easy way.
Cheers,
Wilder
> On 02 Jul 2015, at 20:35, John Burwell <jo...@shapeblue.com> wrote:
>
> Wilder,
>
> In the grand scheme of the entire project history (e.g. reading git log), why do I care about these discrete operations? In six months (or long), I (as the consumer of your change) want to know what motivated this change which is completely lost in those two commits. I have found this advice [1] for commit messages combined with squashing commits to a topic (e.g. defect ticket, feature, enhancement ticket, etc) yields a git history that incredible value over the long term in a projects with a lot of developers making many changes.
>
> Thanks,
> -John
>
> P.S. As an aside, if you encounter a 5000 class, I would encourage you to decompose it rather than reformat the file.
>
> [1]: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message
>
> ---
> John Burwell (@john_burwell)
> VP of Software Engineering, ShapeBlue
> (571) 403-2411 | +44 20 3603 0542
> http://www.shapeblue.com
>
>
>
>> On Jul 2, 2015, at 2:43 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>>
>> Sateesh and Rajesh,
>>
>> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>>
>> I wil give you the simplest example that could come to my mind to encourage no squash:
>>
>> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
>> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>>
>> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>>
>> Would you like to see it quashed and all messed up? It would be very difficult to review.
>>
>> That’s just a simple example.
>>
>> Cheers,
>> Wilder
>>
>>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>>
>>> +1 for squashing commit
>>>
>>> -----Original Message-----
>>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>>> Sent: Thursday, July 2, 2015 12:14 AM
>>> To: dev@cloudstack.apache.org
>>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>>
>>> All,
>>>
>>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>>
>>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>>
>>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>>
>>> Thanks,
>>> -John
>>>
>>> [1]: https://github.com/graydon/bors
>>>
>>> ---
>>> John Burwell (@john_burwell)
>>> VP of Software Engineering, ShapeBlue
>>> (571) 403-2411 | +44 20 3603 0542
>>> http://www.shapeblue.com
>>>
>>>
>>>
>>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>>
>>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>>
>>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>>
>>>>> Starting Monday June 29th (next monday):
>>>>>
>>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>>> - Direct commit will be reverted
>>>>> - Any committer can merge the PR.
>>>>
>>>> +1
>>>>
>>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>>
>>>> Some suggestions and comments to improve PR reviewing/merging:
>>>>
>>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>>
>>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>>
>>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>>
>>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>>
>>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>>
>>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>>
>>>>> There is no tooling involved, just human agreement.
>>>>>
>>>>> cheers,
>>>>
>>>> Regards,
>>>> Rohit Yadav
>>>> Software Architect, ShapeBlue
>>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>>
>>>>
>>>>
>>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>>
>>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>>
>>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
RE: [PROPOSAL] Commit to master through PR only
Posted by Stephen Turner <St...@citrix.com>.
Personally, if I find a bug, I often go back to find when and why it was introduced, and I find it much more useful to be able to identify a specific changeset rather than a large pull request.
--
Stephen Turner
-----Original Message-----
From: John Burwell [mailto:john.burwell@shapeblue.com]
Sent: 02 July 2015 19:35
To: dev@cloudstack.apache.org
Subject: Re: [PROPOSAL] Commit to master through PR only
Wilder,
In the grand scheme of the entire project history (e.g. reading git log), why do I care about these discrete operations? In six months (or long), I (as the consumer of your change) want to know what motivated this change which is completely lost in those two commits. I have found this advice [1] for commit messages combined with squashing commits to a topic (e.g. defect ticket, feature, enhancement ticket, etc) yields a git history that incredible value over the long term in a projects with a lot of developers making many changes.
Thanks,
-John
P.S. As an aside, if you encounter a 5000 class, I would encourage you to decompose it rather than reformat the file.
[1]: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 2, 2015, at 2:43 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>
> Sateesh and Rajesh,
>
> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>
> I wil give you the simplest example that could come to my mind to encourage no squash:
>
> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>
> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>
> Would you like to see it quashed and all messed up? It would be very difficult to review.
>
> That’s just a simple example.
>
> Cheers,
> Wilder
>
>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>
>> +1 for squashing commit
>>
>> -----Original Message-----
>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>> Sent: Thursday, July 2, 2015 12:14 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>
>> All,
>>
>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>
>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>
>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>
>> Thanks,
>> -John
>>
>> [1]: https://github.com/graydon/bors
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>
>>> Hi,
>>>
>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>
>>> +1
>>>
>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>
>>> Some suggestions and comments to improve PR reviewing/merging:
>>>
>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>
>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>
>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>
>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>
>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>
>>> Regards,
>>> Rohit Yadav
>>> Software Architect, ShapeBlue
>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>
>>>
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by John Burwell <jo...@shapeblue.com>.
Wilder,
In the grand scheme of the entire project history (e.g. reading git log), why do I care about these discrete operations? In six months (or long), I (as the consumer of your change) want to know what motivated this change which is completely lost in those two commits. I have found this advice [1] for commit messages combined with squashing commits to a topic (e.g. defect ticket, feature, enhancement ticket, etc) yields a git history that incredible value over the long term in a projects with a lot of developers making many changes.
Thanks,
-John
P.S. As an aside, if you encounter a 5000 class, I would encourage you to decompose it rather than reformat the file.
[1]: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 2, 2015, at 2:43 AM, Wilder Rodrigues <wr...@schubergphilis.com> wrote:
>
> Sateesh and Rajesh,
>
> It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
>
> I wil give you the simplest example that could come to my mind to encourage no squash:
>
> * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
> * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
>
> Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
>
> Would you like to see it quashed and all messed up? It would be very difficult to review.
>
> That’s just a simple example.
>
> Cheers,
> Wilder
>
>> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>>
>> +1 for squashing commit
>>
>> -----Original Message-----
>> From: John Burwell [mailto:john.burwell@shapeblue.com]
>> Sent: Thursday, July 2, 2015 12:14 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [PROPOSAL] Commit to master through PR only
>>
>> All,
>>
>> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>>
>> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>>
>> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>>
>> Thanks,
>> -John
>>
>> [1]: https://github.com/graydon/bors
>>
>> ---
>> John Burwell (@john_burwell)
>> VP of Software Engineering, ShapeBlue
>> (571) 403-2411 | +44 20 3603 0542
>> http://www.shapeblue.com
>>
>>
>>
>>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>>
>>> Hi,
>>>
>>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>>
>>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>>
>>>> However to get started on the right track. I would like to propose the following easy step:
>>>>
>>>> Starting Monday June 29th (next monday):
>>>>
>>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>>> - Direct commit will be reverted
>>>> - Any committer can merge the PR.
>>>
>>> +1
>>>
>>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>>
>>> Some suggestions and comments to improve PR reviewing/merging:
>>>
>>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>>
>>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>>
>>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>>
>>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>>
>>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>>
>>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>>
>>>> There is no tooling involved, just human agreement.
>>>>
>>>> cheers,
>>>
>>> Regards,
>>> Rohit Yadav
>>> Software Architect, ShapeBlue
>>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>>
>>>
>>>
>>> Find out more about ShapeBlue and our range of CloudStack related services
>>>
>>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>>
>>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Wilder Rodrigues <WR...@schubergphilis.com>.
Sateesh and Rajesh,
It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
I wil give you the simplest example that could come to my mind to encourage no squash:
* I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change.
* I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again.
Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change.
Would you like to see it quashed and all messed up? It would be very difficult to review.
That’s just a simple example.
Cheers,
Wilder
> On 02 Jul 2015, at 07:22, Rajesh Battala <ra...@citrix.com> wrote:
>
> +1 for squashing commit
>
> -----Original Message-----
> From: John Burwell [mailto:john.burwell@shapeblue.com]
> Sent: Thursday, July 2, 2015 12:14 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [PROPOSAL] Commit to master through PR only
>
> All,
>
> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
>
> I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
>
> To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
>
> Thanks,
> -John
>
> [1]: https://github.com/graydon/bors
>
> ---
> John Burwell (@john_burwell)
> VP of Software Engineering, ShapeBlue
> (571) 403-2411 | +44 20 3603 0542
> http://www.shapeblue.com
>
>
>
>> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>>
>> Hi,
>>
>>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>>
>>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>>
>>> However to get started on the right track. I would like to propose the following easy step:
>>>
>>> Starting Monday June 29th (next monday):
>>>
>>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>>> - Direct commit will be reverted
>>> - Any committer can merge the PR.
>>
>> +1
>>
>> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>>
>> Some suggestions and comments to improve PR reviewing/merging:
>>
>> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>>
>> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>>
>> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>>
>> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>>
>> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>>
>>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>>
>>> There is no tooling involved, just human agreement.
>>>
>>> cheers,
>>
>> Regards,
>> Rohit Yadav
>> Software Architect, ShapeBlue
>> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
>> Blog: bhaisaab.org | Twitter: @_bhaisaab
>>
>>
>>
>> Find out more about ShapeBlue and our range of CloudStack related services
>>
>> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
>> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
>> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
>> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
>> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>>
>> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
RE: [PROPOSAL] Commit to master through PR only
Posted by Rajesh Battala <ra...@citrix.com>.
+1 for squashing commit
-----Original Message-----
From: John Burwell [mailto:john.burwell@shapeblue.com]
Sent: Thursday, July 2, 2015 12:14 AM
To: dev@cloudstack.apache.org
Subject: Re: [PROPOSAL] Commit to master through PR only
All,
I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
Thanks,
-John
[1]: https://github.com/graydon/bors
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>
> Hi,
>
>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>
>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>
>> However to get started on the right track. I would like to propose the following easy step:
>>
>> Starting Monday June 29th (next monday):
>>
>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>> - Direct commit will be reverted
>> - Any committer can merge the PR.
>
> +1
>
> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>
> Some suggestions and comments to improve PR reviewing/merging:
>
> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>
> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>
> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>
> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>
> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>
>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>
>> There is no tooling involved, just human agreement.
>>
>> cheers,
>
> Regards,
> Rohit Yadav
> Software Architect, ShapeBlue
> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
> Blog: bhaisaab.org | Twitter: @_bhaisaab
>
>
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
RE: [PROPOSAL] Commit to master through PR only
Posted by Sateesh Chodapuneedi <sa...@citrix.com>.
> -----Original Message-----
> From: John Burwell [mailto:john.burwell@shapeblue.com]
> Sent: Thursday, July 2, 2015 12:14 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [PROPOSAL] Commit to master through PR only
>
> All,
>
> I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult
> bordering on the arbitrary — adding a process complexity and the potential to
> start debating if a particular PR is one type or another.
>
> I agree regarding the fast forward, and feel that all PRs should squashed down
> to one commit. Ultimately, intermediate commits that seem informative in a
> feature branch become noise in a history as large as CloudStack’s.
+1 for squashing to 1 commit
Regards,
Sateesh
>
> To enforce the policy and ensure that PRs are merged in an orderly and correct
> manner (i.e. one at time), I think we should consider adopting a tool such as bors
> [1] to verify that the merge passes all tests and then performs the merge. It
> would some minor modification to require two votes, but I doubt that would
> take much effort to implement. If there is interest, I would happy to make those
> changes for the project.
>
> Thanks,
> -John
>
> [1]: https://github.com/graydon/bors
>
> ---
> John Burwell (@john_burwell)
> VP of Software Engineering, ShapeBlue
> (571) 403-2411 | +44 20 3603 0542
> http://www.shapeblue.com
>
>
>
> > On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
> >
> > Hi,
> >
> >> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com>
> wrote:
> >>
> >> A few of us are in Amsterdam at DevOps days. We are chatting about release
> management procedure.
> >> Remi is working on a set of principles that he will put on the wiki to start a
> [DISCUSS].
> >>
> >> However to get started on the right track. I would like to propose the
> following easy step:
> >>
> >> Starting Monday June 29th (next monday):
> >>
> >> - Only commit through PR will land on master (after a minimum of 2 LGTM
> and green Travis results)
> >> - Direct commit will be reverted
> >> - Any committer can merge the PR.
> >
> > +1
> >
> > I’ve been trying to help close PRs, it was difficult at first but then I found some
> tooling to help me do that. I think it’s certainly do-able without investing a lot of
> effort to do it, perhaps can done everyday or every few days in a week.
> >
> > Some suggestions and comments to improve PR reviewing/merging:
> >
> > - Let's merge the PR commits in a fast forward way instead of doing a branch
> merge that introduces frivolous merge commits. This is one approach to do
> quickly and painlessly:
> >
> > http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-
> way/
> >
> > - Let’s try to send PR around on one issue or one broad issue, or against a JIRA
> ticket; but avoid unrelated sub-systems etc
> >
> > - If there are not many changes (say less than 100-200 lines were changed),
> let's have the changes melded into one commit. This can be done either by the
> PR author or by the committer. The immediate benefit is that all the changes will
> be much easy to port across other branches, easy to view and follow git-log,
> and easy to revert-able.
> >
> > - Certain PRs that are typographical fixes, doc fixes and tooling related fixes -
> so let’s review and merge them if we’ve at least one green review (“LGTM”),
> though changes to CloudStack mgmt server, agent and plugins codebase IMO
> should have at least 2 green reviews (“LGTM”).
> >
> >> Goal being to start having a new practice -everything through PR for
> everyone- which is an easy way to gate our own commits building up to a PR.
> >>
> >> There is no tooling involved, just human agreement.
> >>
> >> cheers,
> >
> > Regards,
> > Rohit Yadav
> > Software Architect, ShapeBlue
> > M. +91 88 262 30892 | rohit.yadav@shapeblue.com
> > Blog: bhaisaab.org | Twitter: @_bhaisaab
> >
> >
> >
> > Find out more about ShapeBlue and our range of CloudStack related services
> >
> > IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-
> build//>
> > CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> > CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> > CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-
> engineering/>
> > CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-
> infrastructure-support/>
> > CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-
> training/>
> >
> > This email and any attachments to it may be confidential and are intended
> solely for the use of the individual to whom it is addressed. Any views or
> opinions expressed are solely those of the author and do not necessarily
> represent those of Shape Blue Ltd or related companies. If you are not the
> intended recipient of this email, you must neither take any action based upon its
> contents, nor copy or show it to anyone. Please contact the sender if you
> believe you have received this email in error. Shape Blue Ltd is a company
> incorporated in England & Wales. ShapeBlue Services India LLP is a company
> incorporated in India and is operated under license from Shape Blue Ltd. Shape
> Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated
> under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered
> by The Republic of South Africa and is traded under license from Shape Blue Ltd.
> ShapeBlue is a registered trademark.
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-
> engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-
> infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-
> training/>
>
> This email and any attachments to it may be confidential and are intended solely
> for the use of the individual to whom it is addressed. Any views or opinions
> expressed are solely those of the author and do not necessarily represent those
> of Shape Blue Ltd or related companies. If you are not the intended recipient of
> this email, you must neither take any action based upon its contents, nor copy or
> show it to anyone. Please contact the sender if you believe you have received
> this email in error. Shape Blue Ltd is a company incorporated in England &
> Wales. ShapeBlue Services India LLP is a company incorporated in India and is
> operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda
> is a company incorporated in Brasil and is operated under license from Shape
> Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South
> Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered
> trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by John Burwell <jo...@shapeblue.com>.
All,
I think we should stick to 2 votes per PR. Defining types of PRs becomes difficult bordering on the arbitrary — adding a process complexity and the potential to start debating if a particular PR is one type or another.
I agree regarding the fast forward, and feel that all PRs should squashed down to one commit. Ultimately, intermediate commits that seem informative in a feature branch become noise in a history as large as CloudStack’s.
To enforce the policy and ensure that PRs are merged in an orderly and correct manner (i.e. one at time), I think we should consider adopting a tool such as bors [1] to verify that the merge passes all tests and then performs the merge. It would some minor modification to require two votes, but I doubt that would take much effort to implement. If there is interest, I would happy to make those changes for the project.
Thanks,
-John
[1]: https://github.com/graydon/bors
---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com
> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
>
> Hi,
>
>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>>
>> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
>> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>>
>> However to get started on the right track. I would like to propose the following easy step:
>>
>> Starting Monday June 29th (next monday):
>>
>> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
>> - Direct commit will be reverted
>> - Any committer can merge the PR.
>
> +1
>
> I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
>
> Some suggestions and comments to improve PR reviewing/merging:
>
> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>
> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
>
> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
>
> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
>
> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
>
>> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>>
>> There is no tooling involved, just human agreement.
>>
>> cheers,
>
> Regards,
> Rohit Yadav
> Software Architect, ShapeBlue
> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
> Blog: bhaisaab.org | Twitter: @_bhaisaab
>
>
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Daan Hoogland <da...@gmail.com>.
I'm afraid I don't agree on some of points here, Rohit.
On Wed, Jul 1, 2015 at 7:48 PM, Rohit Yadav <ro...@shapeblue.com> wrote:
...
> Some suggestions and comments to improve PR reviewing/merging:
>
> - Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
>
> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
Most specifically I am a great proponent of merge commits. The make
very clear that a bunch of work belongs together and was merged at a
certain point in time.
>
> - Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
Now this I strongly agree on with you.
>
> - If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
Only if the change is rally doing only one thing. It is the merge
commit that should make it easily revertible.
>
> - Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
this is one I agree on again :) (score for those keeping: 2-2)
--
Daan
Re: [PROPOSAL] Commit to master through PR only
Posted by Rohit Yadav <ro...@shapeblue.com>.
Hi,
> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>
> A few of us are in Amsterdam at DevOps days. We are chatting about release management procedure.
> Remi is working on a set of principles that he will put on the wiki to start a [DISCUSS].
>
> However to get started on the right track. I would like to propose the following easy step:
>
> Starting Monday June 29th (next monday):
>
> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
> - Direct commit will be reverted
> - Any committer can merge the PR.
+1
I’ve been trying to help close PRs, it was difficult at first but then I found some tooling to help me do that. I think it’s certainly do-able without investing a lot of effort to do it, perhaps can done everyday or every few days in a week.
Some suggestions and comments to improve PR reviewing/merging:
- Let's merge the PR commits in a fast forward way instead of doing a branch merge that introduces frivolous merge commits. This is one approach to do quickly and painlessly:
http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/
- Let’s try to send PR around on one issue or one broad issue, or against a JIRA ticket; but avoid unrelated sub-systems etc
- If there are not many changes (say less than 100-200 lines were changed), let's have the changes melded into one commit. This can be done either by the PR author or by the committer. The immediate benefit is that all the changes will be much easy to port across other branches, easy to view and follow git-log, and easy to revert-able.
- Certain PRs that are typographical fixes, doc fixes and tooling related fixes - so let’s review and merge them if we’ve at least one green review (“LGTM”), though changes to CloudStack mgmt server, agent and plugins codebase IMO should have at least 2 green reviews (“LGTM”).
> Goal being to start having a new practice -everything through PR for everyone- which is an easy way to gate our own commits building up to a PR.
>
> There is no tooling involved, just human agreement.
>
> cheers,
Regards,
Rohit Yadav
Software Architect, ShapeBlue
M. +91 88 262 30892 | rohit.yadav@shapeblue.com
Blog: bhaisaab.org | Twitter: @_bhaisaab
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [PROPOSAL] Commit to master through PR only
Posted by Rene Moser <ma...@renemoser.net>.
Hi
On 25.06.2015 16:38, Sebastien Goasguen wrote:
> - Only commit through PR will land on master (after a minimum of 2 LGTM and green Travis results)
> - Direct commit will be reverted
> - Any committer can merge the PR.
That's the way I used to work. That's fine! :)
One technical benefit is that the master should always be "stable" and
"release ready" because PR will be builded and tested before merge.
Yours
René