You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Daan Hoogland <da...@gmail.com> on 2018/01/08 09:45:22 UTC
[DISCUSS] new way of github working
Devs,
I see a lot of merge master to branch commits appearing on PRs. This is
against prior (non-hard) agreements on how we work. It is getting to be the
daily practice so;
How do we feel about
1. not using merge commits anymore?
2. merging back as a way of solving conflicts?
and
Do we need to make a policy of it or do we let it evolve, at the risk of
having more hard to track feature/version matrices?
--
Daan
Re: [DISCUSS] new way of github working
Posted by Rene Moser <ma...@renemoser.net>.
Hi
On 01/08/2018 10:45 AM, Daan Hoogland wrote:
> Devs,
>
> I see a lot of merge master to branch commits appearing on PRs. This is
> against prior (non-hard) agreements on how we work. It is getting to be the
> daily practice so;
> How do we feel about
> 1. not using merge commits anymore?
> 2. merging back as a way of solving conflicts?
> and
> Do we need to make a policy of it or do we let it evolve, at the risk of
> having more hard to track feature/version matrices?
I am +1 merge commits (when it make sense).
Keep history and be able to identify which commits belongs to what PR.
Also helpful if there are more than one author. It also helps when the
PR contains commit with fixes which can be cherry picked and backported
(e.g. the systemvm PR as an example).
Merge commits are not bad if we used them correctly. However, merge
commits in PRs -> bad
I am -1 "always squash commits". Source code is write once read many,
splitting commits into multiple commits can help understand the code.
René
Re: [DISCUSS] new way of github working
Posted by Will Stevens <ws...@cloudops.com>.
If the new approach is adopted, we will likely have to change the
functionality of the 'tools' here:
https://github.com/apache/cloudstack/tree/master/tools/git
Especially 'git-pr' creates merges with the "would be deprecated" style, so
we will probably want to adapt our tracked tooling to conform with any new
styles we adopt.
Cheers,
*Will Stevens*
CTO
<https://goo.gl/NYZ8KK>
On Mon, Jan 15, 2018 at 3:06 PM, Rafael Weingärtner <
rafaelweingartner@gmail.com> wrote:
> Daan,
>
> Now that master is open for merges again, can we get a feedback here? It
> might be interesting to find a consensus and a standardize way of working
> for everybody before we start merging things in master again …
>
>
>
> On Mon, Jan 8, 2018 at 8:40 PM, Rafael Weingärtner <
> rafaelweingartner@gmail.com> wrote:
>
> > This might be my lack of expertise in Git (Github). I like the merge
> > commits (when merging a PR) because I can easily find the date when
> > something has been introduced to the code base. Of course, this can also
> be
> > achieved through Jira tickets and Github PRs history. This means I would
> > not mind adopting the merge and squash option on Github.
> >
> > On the other hand, regarding the second issue, I prefer the philosophy of
> > single commit PRs; otherwise, it feels pretty hard to track what was
> > introduced by a PR then. I recall seeing PRs with 100+ commits laying
> > around, and I confess that I cannot find myself in the middle of that
> > mayhem..
> >
> > Daan, could you provide more insights on the benefits of having a commit
> > per change in a PR?
> >
> > On Mon, Jan 8, 2018 at 7:30 PM, Daan Hoogland <
> daan.hoogland@shapeblue.com
> > > wrote:
> >
> >> Marc-Aurèle and Rafael, I mean both. I know we used to require the
> first,
> >> to create the release notes in a concise way and we used to ban the
> other
> >> because it leads to unnavigable revision trees. But now that squash and
> >> merge is a functionality of github one might argue that it doesn’t
> matter
> >> anymore. Personally, I am against squashing persé, in any case. I am not
> >> the law (other than during some triathlons) so we jointly might decide
> >> differently.
> >>
> >> On 08/01/2018, 16:50, "Nicolas Vazquez" <Ni...@shapeblue.com>
> >> wrote:
> >>
> >> The same as Rafael described. If it is the second case I would
> prefer
> >> rebasing the target branch and push force instead of including merge
> >> commits in a PR
> >>
> >> Obtener Outlook para Android<https://aka.ms/ghei36>
> >>
> >> ________________________________
> >> From: williamstevens@gmail.com <wi...@gmail.com> on behalf
> >> of Will Stevens <ws...@cloudops.com>
> >> Sent: Monday, January 8, 2018 11:34:42 AM
> >> To: dev@cloudstack.apache.org
> >> Subject: Re: [DISCUSS] new way of github working
> >>
> >> Just a note Daan. If a PR is merged with the `git pr ####` tool in
> >> our
> >> utilities folder, it will automatically include the merge commit.
> >> Figured
> >> I should mention that...
> >>
> >> *Will Stevens*
> >> CTO
> >>
> >> <https://goo.gl/NYZ8KK>
> >>
> >> On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <
> >> marco@exoscale.ch>
> >> wrote:
> >>
> >> > Same opinion as Rafael described.
> >> >
> >> > On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
> >> > rafaelweingartner@gmail.com> wrote:
> >> >
> >> > > I did not fully understand what you meant.
> >> > >
> >> > > Are you talking about the merge commit that can be created when
> a
> >> PR is
> >> > > merged? Or, are you talking about a merge commit that is added
> to
> >> a PR
> >> > when
> >> > > a conflict is solved by its author(s)?
> >> > >
> >> > >
> >> > > I do not have problems with the first type of merge commits.
> >> However, I
> >> > > think we should not allow the second type to get into our code
> >> base.
> >> > >
> >> > > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <
> >> daan.hoogland@gmail.com>
> >> > > wrote:
> >> > >
> >> > > > Devs,
> >> > > >
> >> > > > I see a lot of merge master to branch commits appearing on
> PRs.
> >> This is
> >> > > > against prior (non-hard) agreements on how we work. It is
> >> getting to be
> >> > > the
> >> > > > daily practice so;
> >> > > > How do we feel about
> >> > > > 1. not using merge commits anymore?
> >> > > > 2. merging back as a way of solving conflicts?
> >> > > > and
> >> > > > Do we need to make a policy of it or do we let it evolve, at
> >> the risk
> >> > of
> >> > > > having more hard to track feature/version matrices?
> >> > > >
> >> > > > --
> >> > > > Daan
> >> > > >
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > > Rafael Weingärtner
> >> > >
> >> >
> >>
> >> Nicolas.Vazquez@shapeblue.com
> >> www.shapeblue.com
> >> ,
> >> @shapeblue
> >>
> >>
> >>
> >>
> >>
> >>
> >> daan.hoogland@shapeblue.com
> >> www.shapeblue.com
> >> 53 Chandos Place, Covent Garden, London WC2N 4HSUK
> >> @shapeblue
> >>
> >>
> >>
> >>
> >
> >
> > --
> > Rafael Weingärtner
> >
>
>
>
> --
> Rafael Weingärtner
>
Re: [DISCUSS] new way of github working
Posted by Rohit Yadav <ro...@shapeblue.com>.
My preference is to not squash commits when a PR is created, I do prefer squash merge of PR. Also, instead of a rule I would prefer this to be a general guidelines, as there may be cases where it may be valid to have few commits in which case I will prefer doing a rebase+merge (via github or otherwise). As an example, if a PR is worked on by several people (say it's a big change, or someone carries forward an old PR whose primary author is not responding), I can prefer to have commits melded by author (to give credit to authors involved) or component (say on test, scripts/infra, java packages,).
I think during course of PR review, not squashing commits helps reviewers to track what got changed from the initial PR.
Merging of PR with an explicit merge commit is something I don't like as it messes up the git history.
- Rohit
<https://cloudstack.apache.org>
________________________________
From: Rafael Weingärtner <ra...@gmail.com>
Sent: Wednesday, May 2, 2018 4:53:41 PM
To: dev
Subject: Re: [DISCUSS] new way of github working
I think we are on the same page here. There is only one thing that is
different from what we are doing in PRs.
> Before you create a PR, squash all applicable commits to make it more
> readable for reviewers
>
I would suggest not squashing everything before creating the PR. I mean, do
the following when coding:
- disable your auto formatter (in Eclipse you can just disable the save
actions) if you are going to change a class that might need formatting
- do the changes
- create a commit for the main changes of the PR
- enable the auto formatter and formatting the change files
- create a commit for formatting and cleanup changes
- then create the PR
Separating formatting/cleanups commits makes it easier for reviewers to
evaluate changes.
On Tue, May 1, 2018 at 3:53 PM, Tutkowski, Mike <Mi...@netapp.com>
wrote:
> Hi everyone,
>
> We had a good conversation going here. Maybe we can continue it, get some
> level of reasonable consensus, and implement it (if, in fact, the consensus
> is a change from what we currently have).
>
> My suggested approach is the following:
>
> Before you create a PR, squash all applicable commits to make it more
> readable for reviewers. Once reviews start coming in and you start making
> changes, push new commits on top of the prior ones (do not squash at this
> point). This will make it easier for reviewers to confirm that you and they
> are on the same page with regards to what was changed. When you need to
> draw in changes from the base branch, rebase your commits on top of it.
> When the PR is given a LGTM by 2+ reviewers and passed the necessary
> regression tests, it should be squashed and then merged. I see the
> evolution of commits during the life of the PR as a temporary sandbox of
> history that is no longer required once the PR has been completed.
>
> I think that process should cover the vast majority of our PRs.
>
> There are usually some exceptions to the rule, however. When this happens,
> discuss your situation with the reviewers and bring any concerns to the
> mailing list before deviating from the standard process.
>
> Thoughts?
> Mike
>
> On 1/15/18, 1:47 PM, "Rene Moser" <ma...@renemoser.net> wrote:
>
>
>
> On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> > Daan,
> >
> > Now that master is open for merges again, can we get a feedback
> here? It
> > might be interesting to find a consensus and a standardize way of
> working
> > for everybody before we start merging things in master again …
>
> +1 to allow merge commits on master branch to keep history of a series
> of patches when they help to understand the change.
>
> René
>
>
>
>
>
--
Rafael Weingärtner
rohit.yadav@shapeblue.com
www.shapeblue.com
53 Chandos Place, Covent Garden, London WC2N 4HSUK
@shapeblue
Re: [DISCUSS] new way of github working
Posted by Rafael Weingärtner <ra...@gmail.com>.
I think we are on the same page here. There is only one thing that is
different from what we are doing in PRs.
> Before you create a PR, squash all applicable commits to make it more
> readable for reviewers
>
I would suggest not squashing everything before creating the PR. I mean, do
the following when coding:
- disable your auto formatter (in Eclipse you can just disable the save
actions) if you are going to change a class that might need formatting
- do the changes
- create a commit for the main changes of the PR
- enable the auto formatter and formatting the change files
- create a commit for formatting and cleanup changes
- then create the PR
Separating formatting/cleanups commits makes it easier for reviewers to
evaluate changes.
On Tue, May 1, 2018 at 3:53 PM, Tutkowski, Mike <Mi...@netapp.com>
wrote:
> Hi everyone,
>
> We had a good conversation going here. Maybe we can continue it, get some
> level of reasonable consensus, and implement it (if, in fact, the consensus
> is a change from what we currently have).
>
> My suggested approach is the following:
>
> Before you create a PR, squash all applicable commits to make it more
> readable for reviewers. Once reviews start coming in and you start making
> changes, push new commits on top of the prior ones (do not squash at this
> point). This will make it easier for reviewers to confirm that you and they
> are on the same page with regards to what was changed. When you need to
> draw in changes from the base branch, rebase your commits on top of it.
> When the PR is given a LGTM by 2+ reviewers and passed the necessary
> regression tests, it should be squashed and then merged. I see the
> evolution of commits during the life of the PR as a temporary sandbox of
> history that is no longer required once the PR has been completed.
>
> I think that process should cover the vast majority of our PRs.
>
> There are usually some exceptions to the rule, however. When this happens,
> discuss your situation with the reviewers and bring any concerns to the
> mailing list before deviating from the standard process.
>
> Thoughts?
> Mike
>
> On 1/15/18, 1:47 PM, "Rene Moser" <ma...@renemoser.net> wrote:
>
>
>
> On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> > Daan,
> >
> > Now that master is open for merges again, can we get a feedback
> here? It
> > might be interesting to find a consensus and a standardize way of
> working
> > for everybody before we start merging things in master again …
>
> +1 to allow merge commits on master branch to keep history of a series
> of patches when they help to understand the change.
>
> René
>
>
>
>
>
--
Rafael Weingärtner
Re: [DISCUSS] new way of github working
Posted by "Tutkowski, Mike" <Mi...@netapp.com>.
Hi everyone,
We had a good conversation going here. Maybe we can continue it, get some level of reasonable consensus, and implement it (if, in fact, the consensus is a change from what we currently have).
My suggested approach is the following:
Before you create a PR, squash all applicable commits to make it more readable for reviewers. Once reviews start coming in and you start making changes, push new commits on top of the prior ones (do not squash at this point). This will make it easier for reviewers to confirm that you and they are on the same page with regards to what was changed. When you need to draw in changes from the base branch, rebase your commits on top of it. When the PR is given a LGTM by 2+ reviewers and passed the necessary regression tests, it should be squashed and then merged. I see the evolution of commits during the life of the PR as a temporary sandbox of history that is no longer required once the PR has been completed.
I think that process should cover the vast majority of our PRs.
There are usually some exceptions to the rule, however. When this happens, discuss your situation with the reviewers and bring any concerns to the mailing list before deviating from the standard process.
Thoughts?
Mike
On 1/15/18, 1:47 PM, "Rene Moser" <ma...@renemoser.net> wrote:
On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> Daan,
>
> Now that master is open for merges again, can we get a feedback here? It
> might be interesting to find a consensus and a standardize way of working
> for everybody before we start merging things in master again …
+1 to allow merge commits on master branch to keep history of a series
of patches when they help to understand the change.
René
Re: [DISCUSS] new way of github working
Posted by Rafael Weingärtner <ra...@gmail.com>.
These last two months I have experimented working with both ways
(maintaining the merge commit and squashing and merging). When I
experimented using “squash+merge”, the only time I was maintaining the
merge commit is during merge-forwards.
I liked working with “squash+merge” approach. This allowed me to split my
PRs into multiple commits, which facilitates for reviewers. For instance,
it is interesting to split code changes and formatting ones. Moreover,
during the review of the PR, we can create a history of changes that were
created because of reviews. Then, when everything is ready to be merged, we
can squash everything (using Github UI), and merge the PR as a single
commit. Of course, to adopt this method we need to present single changes
per PR. Meaning, a PR should not address multiple issues at the same time.
One thing we need to change though. When merging forward we must customize
the “forward merge” message. Otherwise, we end up with a bunch of commits
that have the same commit title (e.g. “merge 4.11”). We can use messages
such as “Froward merge #<prNumber> from <branch>”, and then add in the body
of the commit the title of the commit that was used to merge the
#<prNumber>.
What do you guys think? Let’s finalize this discussion and define a
standard to adopt. Then, I can change the merging tools (are people using
them after we moved to Github?) accordingly (if needed). I can also update
the merge document that is on our wiki page.
On Mon, Jan 15, 2018 at 6:47 PM, Rene Moser <ma...@renemoser.net> wrote:
>
>
> On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> > Daan,
> >
> > Now that master is open for merges again, can we get a feedback here? It
> > might be interesting to find a consensus and a standardize way of working
> > for everybody before we start merging things in master again …
>
> +1 to allow merge commits on master branch to keep history of a series
> of patches when they help to understand the change.
>
> René
>
>
>
--
Rafael Weingärtner
Re: [DISCUSS] new way of github working
Posted by Rene Moser <ma...@renemoser.net>.
On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> Daan,
>
> Now that master is open for merges again, can we get a feedback here? It
> might be interesting to find a consensus and a standardize way of working
> for everybody before we start merging things in master again …
+1 to allow merge commits on master branch to keep history of a series
of patches when they help to understand the change.
René
Re: [DISCUSS] new way of github working
Posted by Rafael Weingärtner <ra...@gmail.com>.
Daan,
Now that master is open for merges again, can we get a feedback here? It
might be interesting to find a consensus and a standardize way of working
for everybody before we start merging things in master again …
On Mon, Jan 8, 2018 at 8:40 PM, Rafael Weingärtner <
rafaelweingartner@gmail.com> wrote:
> This might be my lack of expertise in Git (Github). I like the merge
> commits (when merging a PR) because I can easily find the date when
> something has been introduced to the code base. Of course, this can also be
> achieved through Jira tickets and Github PRs history. This means I would
> not mind adopting the merge and squash option on Github.
>
> On the other hand, regarding the second issue, I prefer the philosophy of
> single commit PRs; otherwise, it feels pretty hard to track what was
> introduced by a PR then. I recall seeing PRs with 100+ commits laying
> around, and I confess that I cannot find myself in the middle of that
> mayhem..
>
> Daan, could you provide more insights on the benefits of having a commit
> per change in a PR?
>
> On Mon, Jan 8, 2018 at 7:30 PM, Daan Hoogland <daan.hoogland@shapeblue.com
> > wrote:
>
>> Marc-Aurèle and Rafael, I mean both. I know we used to require the first,
>> to create the release notes in a concise way and we used to ban the other
>> because it leads to unnavigable revision trees. But now that squash and
>> merge is a functionality of github one might argue that it doesn’t matter
>> anymore. Personally, I am against squashing persé, in any case. I am not
>> the law (other than during some triathlons) so we jointly might decide
>> differently.
>>
>> On 08/01/2018, 16:50, "Nicolas Vazquez" <Ni...@shapeblue.com>
>> wrote:
>>
>> The same as Rafael described. If it is the second case I would prefer
>> rebasing the target branch and push force instead of including merge
>> commits in a PR
>>
>> Obtener Outlook para Android<https://aka.ms/ghei36>
>>
>> ________________________________
>> From: williamstevens@gmail.com <wi...@gmail.com> on behalf
>> of Will Stevens <ws...@cloudops.com>
>> Sent: Monday, January 8, 2018 11:34:42 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [DISCUSS] new way of github working
>>
>> Just a note Daan. If a PR is merged with the `git pr ####` tool in
>> our
>> utilities folder, it will automatically include the merge commit.
>> Figured
>> I should mention that...
>>
>> *Will Stevens*
>> CTO
>>
>> <https://goo.gl/NYZ8KK>
>>
>> On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <
>> marco@exoscale.ch>
>> wrote:
>>
>> > Same opinion as Rafael described.
>> >
>> > On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
>> > rafaelweingartner@gmail.com> wrote:
>> >
>> > > I did not fully understand what you meant.
>> > >
>> > > Are you talking about the merge commit that can be created when a
>> PR is
>> > > merged? Or, are you talking about a merge commit that is added to
>> a PR
>> > when
>> > > a conflict is solved by its author(s)?
>> > >
>> > >
>> > > I do not have problems with the first type of merge commits.
>> However, I
>> > > think we should not allow the second type to get into our code
>> base.
>> > >
>> > > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <
>> daan.hoogland@gmail.com>
>> > > wrote:
>> > >
>> > > > Devs,
>> > > >
>> > > > I see a lot of merge master to branch commits appearing on PRs.
>> This is
>> > > > against prior (non-hard) agreements on how we work. It is
>> getting to be
>> > > the
>> > > > daily practice so;
>> > > > How do we feel about
>> > > > 1. not using merge commits anymore?
>> > > > 2. merging back as a way of solving conflicts?
>> > > > and
>> > > > Do we need to make a policy of it or do we let it evolve, at
>> the risk
>> > of
>> > > > having more hard to track feature/version matrices?
>> > > >
>> > > > --
>> > > > Daan
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Rafael Weingärtner
>> > >
>> >
>>
>> Nicolas.Vazquez@shapeblue.com
>> www.shapeblue.com
>> ,
>> @shapeblue
>>
>>
>>
>>
>>
>>
>> daan.hoogland@shapeblue.com
>> www.shapeblue.com
>> 53 Chandos Place, Covent Garden, London WC2N 4HSUK
>> @shapeblue
>>
>>
>>
>>
>
>
> --
> Rafael Weingärtner
>
--
Rafael Weingärtner
Re: [DISCUSS] new way of github working
Posted by Rafael Weingärtner <ra...@gmail.com>.
This might be my lack of expertise in Git (Github). I like the merge
commits (when merging a PR) because I can easily find the date when
something has been introduced to the code base. Of course, this can also be
achieved through Jira tickets and Github PRs history. This means I would
not mind adopting the merge and squash option on Github.
On the other hand, regarding the second issue, I prefer the philosophy of
single commit PRs; otherwise, it feels pretty hard to track what was
introduced by a PR then. I recall seeing PRs with 100+ commits laying
around, and I confess that I cannot find myself in the middle of that
mayhem..
Daan, could you provide more insights on the benefits of having a commit
per change in a PR?
On Mon, Jan 8, 2018 at 7:30 PM, Daan Hoogland <da...@shapeblue.com>
wrote:
> Marc-Aurèle and Rafael, I mean both. I know we used to require the first,
> to create the release notes in a concise way and we used to ban the other
> because it leads to unnavigable revision trees. But now that squash and
> merge is a functionality of github one might argue that it doesn’t matter
> anymore. Personally, I am against squashing persé, in any case. I am not
> the law (other than during some triathlons) so we jointly might decide
> differently.
>
> On 08/01/2018, 16:50, "Nicolas Vazquez" <Ni...@shapeblue.com>
> wrote:
>
> The same as Rafael described. If it is the second case I would prefer
> rebasing the target branch and push force instead of including merge
> commits in a PR
>
> Obtener Outlook para Android<https://aka.ms/ghei36>
>
> ________________________________
> From: williamstevens@gmail.com <wi...@gmail.com> on behalf
> of Will Stevens <ws...@cloudops.com>
> Sent: Monday, January 8, 2018 11:34:42 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [DISCUSS] new way of github working
>
> Just a note Daan. If a PR is merged with the `git pr ####` tool in our
> utilities folder, it will automatically include the merge commit.
> Figured
> I should mention that...
>
> *Will Stevens*
> CTO
>
> <https://goo.gl/NYZ8KK>
>
> On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <
> marco@exoscale.ch>
> wrote:
>
> > Same opinion as Rafael described.
> >
> > On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
> > rafaelweingartner@gmail.com> wrote:
> >
> > > I did not fully understand what you meant.
> > >
> > > Are you talking about the merge commit that can be created when a
> PR is
> > > merged? Or, are you talking about a merge commit that is added to
> a PR
> > when
> > > a conflict is solved by its author(s)?
> > >
> > >
> > > I do not have problems with the first type of merge commits.
> However, I
> > > think we should not allow the second type to get into our code
> base.
> > >
> > > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <
> daan.hoogland@gmail.com>
> > > wrote:
> > >
> > > > Devs,
> > > >
> > > > I see a lot of merge master to branch commits appearing on PRs.
> This is
> > > > against prior (non-hard) agreements on how we work. It is
> getting to be
> > > the
> > > > daily practice so;
> > > > How do we feel about
> > > > 1. not using merge commits anymore?
> > > > 2. merging back as a way of solving conflicts?
> > > > and
> > > > Do we need to make a policy of it or do we let it evolve, at the
> risk
> > of
> > > > having more hard to track feature/version matrices?
> > > >
> > > > --
> > > > Daan
> > > >
> > >
> > >
> > >
> > > --
> > > Rafael Weingärtner
> > >
> >
>
> Nicolas.Vazquez@shapeblue.com
> www.shapeblue.com
> ,
> @shapeblue
>
>
>
>
>
>
> daan.hoogland@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London WC2N 4HSUK
> @shapeblue
>
>
>
>
--
Rafael Weingärtner
Re: [DISCUSS] new way of github working
Posted by Daan Hoogland <da...@shapeblue.com>.
Marc-Aurèle and Rafael, I mean both. I know we used to require the first, to create the release notes in a concise way and we used to ban the other because it leads to unnavigable revision trees. But now that squash and merge is a functionality of github one might argue that it doesn’t matter anymore. Personally, I am against squashing persé, in any case. I am not the law (other than during some triathlons) so we jointly might decide differently.
On 08/01/2018, 16:50, "Nicolas Vazquez" <Ni...@shapeblue.com> wrote:
The same as Rafael described. If it is the second case I would prefer rebasing the target branch and push force instead of including merge commits in a PR
Obtener Outlook para Android<https://aka.ms/ghei36>
________________________________
From: williamstevens@gmail.com <wi...@gmail.com> on behalf of Will Stevens <ws...@cloudops.com>
Sent: Monday, January 8, 2018 11:34:42 AM
To: dev@cloudstack.apache.org
Subject: Re: [DISCUSS] new way of github working
Just a note Daan. If a PR is merged with the `git pr ####` tool in our
utilities folder, it will automatically include the merge commit. Figured
I should mention that...
*Will Stevens*
CTO
<https://goo.gl/NYZ8KK>
On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <ma...@exoscale.ch>
wrote:
> Same opinion as Rafael described.
>
> On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
> rafaelweingartner@gmail.com> wrote:
>
> > I did not fully understand what you meant.
> >
> > Are you talking about the merge commit that can be created when a PR is
> > merged? Or, are you talking about a merge commit that is added to a PR
> when
> > a conflict is solved by its author(s)?
> >
> >
> > I do not have problems with the first type of merge commits. However, I
> > think we should not allow the second type to get into our code base.
> >
> > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <da...@gmail.com>
> > wrote:
> >
> > > Devs,
> > >
> > > I see a lot of merge master to branch commits appearing on PRs. This is
> > > against prior (non-hard) agreements on how we work. It is getting to be
> > the
> > > daily practice so;
> > > How do we feel about
> > > 1. not using merge commits anymore?
> > > 2. merging back as a way of solving conflicts?
> > > and
> > > Do we need to make a policy of it or do we let it evolve, at the risk
> of
> > > having more hard to track feature/version matrices?
> > >
> > > --
> > > Daan
> > >
> >
> >
> >
> > --
> > Rafael Weingärtner
> >
>
Nicolas.Vazquez@shapeblue.com
www.shapeblue.com
,
@shapeblue
daan.hoogland@shapeblue.com
www.shapeblue.com
53 Chandos Place, Covent Garden, London WC2N 4HSUK
@shapeblue
Re: [DISCUSS] new way of github working
Posted by Nicolas Vazquez <Ni...@shapeblue.com>.
The same as Rafael described. If it is the second case I would prefer rebasing the target branch and push force instead of including merge commits in a PR
Obtener Outlook para Android<https://aka.ms/ghei36>
________________________________
From: williamstevens@gmail.com <wi...@gmail.com> on behalf of Will Stevens <ws...@cloudops.com>
Sent: Monday, January 8, 2018 11:34:42 AM
To: dev@cloudstack.apache.org
Subject: Re: [DISCUSS] new way of github working
Just a note Daan. If a PR is merged with the `git pr ####` tool in our
utilities folder, it will automatically include the merge commit. Figured
I should mention that...
*Will Stevens*
CTO
<https://goo.gl/NYZ8KK>
On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <ma...@exoscale.ch>
wrote:
> Same opinion as Rafael described.
>
> On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
> rafaelweingartner@gmail.com> wrote:
>
> > I did not fully understand what you meant.
> >
> > Are you talking about the merge commit that can be created when a PR is
> > merged? Or, are you talking about a merge commit that is added to a PR
> when
> > a conflict is solved by its author(s)?
> >
> >
> > I do not have problems with the first type of merge commits. However, I
> > think we should not allow the second type to get into our code base.
> >
> > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <da...@gmail.com>
> > wrote:
> >
> > > Devs,
> > >
> > > I see a lot of merge master to branch commits appearing on PRs. This is
> > > against prior (non-hard) agreements on how we work. It is getting to be
> > the
> > > daily practice so;
> > > How do we feel about
> > > 1. not using merge commits anymore?
> > > 2. merging back as a way of solving conflicts?
> > > and
> > > Do we need to make a policy of it or do we let it evolve, at the risk
> of
> > > having more hard to track feature/version matrices?
> > >
> > > --
> > > Daan
> > >
> >
> >
> >
> > --
> > Rafael Weingärtner
> >
>
Nicolas.Vazquez@shapeblue.com
www.shapeblue.com
,
@shapeblue
Re: [DISCUSS] new way of github working
Posted by Will Stevens <ws...@cloudops.com>.
Just a note Daan. If a PR is merged with the `git pr ####` tool in our
utilities folder, it will automatically include the merge commit. Figured
I should mention that...
*Will Stevens*
CTO
<https://goo.gl/NYZ8KK>
On Mon, Jan 8, 2018 at 8:26 AM, Marc-Aurèle Brothier <ma...@exoscale.ch>
wrote:
> Same opinion as Rafael described.
>
> On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
> rafaelweingartner@gmail.com> wrote:
>
> > I did not fully understand what you meant.
> >
> > Are you talking about the merge commit that can be created when a PR is
> > merged? Or, are you talking about a merge commit that is added to a PR
> when
> > a conflict is solved by its author(s)?
> >
> >
> > I do not have problems with the first type of merge commits. However, I
> > think we should not allow the second type to get into our code base.
> >
> > On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <da...@gmail.com>
> > wrote:
> >
> > > Devs,
> > >
> > > I see a lot of merge master to branch commits appearing on PRs. This is
> > > against prior (non-hard) agreements on how we work. It is getting to be
> > the
> > > daily practice so;
> > > How do we feel about
> > > 1. not using merge commits anymore?
> > > 2. merging back as a way of solving conflicts?
> > > and
> > > Do we need to make a policy of it or do we let it evolve, at the risk
> of
> > > having more hard to track feature/version matrices?
> > >
> > > --
> > > Daan
> > >
> >
> >
> >
> > --
> > Rafael Weingärtner
> >
>
Re: [DISCUSS] new way of github working
Posted by Marc-Aurèle Brothier <ma...@exoscale.ch>.
Same opinion as Rafael described.
On Mon, Jan 8, 2018 at 11:30 AM, Rafael Weingärtner <
rafaelweingartner@gmail.com> wrote:
> I did not fully understand what you meant.
>
> Are you talking about the merge commit that can be created when a PR is
> merged? Or, are you talking about a merge commit that is added to a PR when
> a conflict is solved by its author(s)?
>
>
> I do not have problems with the first type of merge commits. However, I
> think we should not allow the second type to get into our code base.
>
> On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <da...@gmail.com>
> wrote:
>
> > Devs,
> >
> > I see a lot of merge master to branch commits appearing on PRs. This is
> > against prior (non-hard) agreements on how we work. It is getting to be
> the
> > daily practice so;
> > How do we feel about
> > 1. not using merge commits anymore?
> > 2. merging back as a way of solving conflicts?
> > and
> > Do we need to make a policy of it or do we let it evolve, at the risk of
> > having more hard to track feature/version matrices?
> >
> > --
> > Daan
> >
>
>
>
> --
> Rafael Weingärtner
>
Re: [DISCUSS] new way of github working
Posted by Rafael Weingärtner <ra...@gmail.com>.
I did not fully understand what you meant.
Are you talking about the merge commit that can be created when a PR is
merged? Or, are you talking about a merge commit that is added to a PR when
a conflict is solved by its author(s)?
I do not have problems with the first type of merge commits. However, I
think we should not allow the second type to get into our code base.
On Mon, Jan 8, 2018 at 7:45 AM, Daan Hoogland <da...@gmail.com>
wrote:
> Devs,
>
> I see a lot of merge master to branch commits appearing on PRs. This is
> against prior (non-hard) agreements on how we work. It is getting to be the
> daily practice so;
> How do we feel about
> 1. not using merge commits anymore?
> 2. merging back as a way of solving conflicts?
> and
> Do we need to make a policy of it or do we let it evolve, at the risk of
> having more hard to track feature/version matrices?
>
> --
> Daan
>
--
Rafael Weingärtner