Github has added supported for merging PRs via squash commit in addition to
the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no "Merge pull request" commits
in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose either
option or restrict it to a single option. I am proposing a switch to
"squash commit" as the only option on the primary Katello repositories. I
would like to get feedback from committers with either a '+1' for agreement
or discussion as to the prons/cons of this approach.
Thanks
···
--
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
> All,
>
> Github has added supported for merging PRs via squash commit in addition to
> the traditional merge request. This means:
>
> 1) If multiple commits in a merge request, they are squash into 1 (i.e.
> fairly standard for acceptance criteria)
> 2) Commits are merged a single commit with no "Merge pull request" commits
> in the repository
>
> Katello has historically used the merge pull request button instead of
> committing and pushing to achieve a linear history. The squash commit, I
> believe, would allow for linear history from this point forward.
>
> Github lets repositories be configured to allow committers to choose either
> option or restrict it to a single option. I am proposing a switch to
> "squash commit" as the only option on the primary Katello repositories. I
> would like to get feedback from committers with either a '+1' for agreement
> or discussion as to the prons/cons of this approach.
>
So in tig, it would be something like
Style #1 (current style)
merge of XYZ
>
> * XYZ
>
older commits
vs Style #2 (proposed new style)
XYZ
>
older commits
If this is the case, then I am in favor of style #2.
> From: "Eric D Helms" <ericdhelms@gmail.com>
> To: "foreman-dev" <foreman-dev@googlegroups.com>
> Sent: Tuesday, April 5, 2016 1:22:02 PM
> Subject: [foreman-dev] Katello PRs and merge vs. squash commits
>
> All,
>
> Github has added supported for merging PRs via squash commit in addition to
> the traditional merge request. This means:
>
> 1) If multiple commits in a merge request, they are squash into 1 (i.e.
> fairly standard for acceptance criteria)
> 2) Commits are merged a single commit with no "Merge pull request" commits
> in the repository
>
> Katello has historically used the merge pull request button instead of
> committing and pushing to achieve a linear history. The squash commit, I
> believe, would allow for linear history from this point forward.
>
> Github lets repositories be configured to allow committers to choose either
> option or restrict it to a single option. I am proposing a switch to
> "squash commit" as the only option on the primary Katello repositories. I
> would like to get feedback from committers with either a '+1' for agreement
> or discussion as to the prons/cons of this approach.
+1
Yes, definitely. I'd like to see Foreman and Katello adopt the same commit history
style.
Beyond that, how about no longer manually squash and force pushing PR's? Just add
additional commits, as they'll be squashed by the Big Green Button Clicker. It will
need changes to the PR processor, because I believe the squash commit message is
generated from the PR title, so that's all it should need to check.
IHMO it makes the review process so much easier. If I make some comments on the PR,
and then the PR author makes those changes as an additional commit, I can easily
review what changed.
···
----- Original Message -----
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
I do prefer the squash commit option but I think there have been rare times
when we wanted to merge a PR that had multiple commits without squashing
them into a single commit (e.g. rails 4 work). I suppose we could either
temporarily re-enable the merge commit option though if the need ever
arises.
David
···
On Tue, Apr 5, 2016 at 1:22 PM, Eric D Helms wrote:
All,
Github has added supported for merging PRs via squash commit in addition
to the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no "Merge pull request"
commits in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose
either option or restrict it to a single option. I am proposing a switch to
"squash commit" as the only option on the primary Katello repositories. I
would like to get feedback from committers with either a ‘+1’ for agreement
or discussion as to the prons/cons of this approach.
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
> Github lets repositories be configured to allow committers to choose either
> option or restrict it to a single option. I am proposing a switch to
> "squash commit" as the only option on the primary Katello repositories. I
> would like to get feedback from committers with either a '+1' for agreement
> or discussion as to the prons/cons of this approach.
I kinda missed the announcement, for the record we are talking about
I love the feature, actually I asked github years ago and got a reply
they are already tracking that. Took some time, but we finally have it.
The date confused me a bit, but the checkbox is really there!
I agree with stacking commits, just let's agree that the PR subject must
be in the expected form "Fixes #XYZ - blah" which will be then used for
the commit message.
I will likely keep using my rebase script when merging, I will just need
to do some changes to align with our agreement:
I am against squash commit as the only option. Squashing sometimes makes sense, usually only when there is one person who owns the feature or fix. But more than once I've worked on a branch with other people making commits. In those pull-requests we've been asked to squash the commit down to one which is a bit of a slap in the face to me. Multiple people have contributed to a given feature or fix, but squashing commits only allows for one name attached to the changes, removing authorship and credit from the others who contributed. I certainly don't want my name removed from code I've written and I'd assume a similar sentiment from others?
If we're wanting to use Github's squash commit feature, I'm apprehensive but I guess it's alright. What I would prefer is the choice of both because there are occasions where it makes more sense to leave multiple commits in a pull-request.
···
----- Original Message -----
> All,
>
> Github has added supported for merging PRs via squash commit in addition to
> the traditional merge request. This means:
>
> 1) If multiple commits in a merge request, they are squash into 1 (i.e.
> fairly standard for acceptance criteria)
> 2) Commits are merged a single commit with no "Merge pull request" commits
> in the repository
>
> Katello has historically used the merge pull request button instead of
> committing and pushing to achieve a linear history. The squash commit, I
> believe, would allow for linear history from this point forward.
>
> Github lets repositories be configured to allow committers to choose either
> option or restrict it to a single option. I am proposing a switch to
> "squash commit" as the only option on the primary Katello repositories. I
> would like to get feedback from committers with either a '+1' for agreement
> or discussion as to the prons/cons of this approach.
>
>
> Thanks
>
> --
> Eric D. Helms
> Red Hat Engineering
> Ph.D. Student - North Carolina State University
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
> I do prefer the squash commit option but I think there have been rare
> times when we wanted to merge a PR that had multiple commits without
> squashing them into a single commit (e.g. rails 4 work). I suppose we could
> either temporarily re-enable the merge commit option though if the need
> ever arises.
>
>
It does help to reinforce not taking this development approach, which I
like
Eric
···
On Tue, Apr 5, 2016 at 3:51 PM, David Davis wrote:
Github has added supported for merging PRs via squash commit in addition
to the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no "Merge pull request"
commits in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose
either option or restrict it to a single option. I am proposing a switch to
"squash commit" as the only option on the primary Katello repositories. I
would like to get feedback from committers with either a ‘+1’ for agreement
or discussion as to the prons/cons of this approach.
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
Something to be aware of is that the final commit message is based on the
PR title plus the commit messages of the commits being squashed. You can
however customize the message when you merge.
David
···
On Tue, Apr 5, 2016 at 3:13 PM, Stephen Benjamin wrote:
----- Original Message -----
From: “Eric D Helms” ericdhelms@gmail.com
To: “foreman-dev” foreman-dev@googlegroups.com
Sent: Tuesday, April 5, 2016 1:22:02 PM
Subject: [foreman-dev] Katello PRs and merge vs. squash commits
All,
Github has added supported for merging PRs via squash commit in addition
to
the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no "Merge pull request"
commits
in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose
either
option or restrict it to a single option. I am proposing a switch to
"squash commit" as the only option on the primary Katello repositories. I
would like to get feedback from committers with either a ‘+1’ for
agreement
or discussion as to the prons/cons of this approach.
+1
Yes, definitely. I’d like to see Foreman and Katello adopt the same
commit history
style.
Beyond that, how about no longer manually squash and force pushing PR’s?
Just add
additional commits, as they’ll be squashed by the Big Green Button
Clicker. It will
need changes to the PR processor, because I believe the squash commit
message is
generated from the PR title, so that’s all it should need to check.
IHMO it makes the review process so much easier. If I make some comments
on the PR,
and then the PR author makes those changes as an additional commit, I can
easily
review what changed.
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
> > All,
> >
> > Github has added supported for merging PRs via squash commit in addition to
> > the traditional merge request. This means:
> >
> > 1) If multiple commits in a merge request, they are squash into 1 (i.e.
> > fairly standard for acceptance criteria)
> > 2) Commits are merged a single commit with no "Merge pull request" commits
> > in the repository
> >
> > Katello has historically used the merge pull request button instead of
> > committing and pushing to achieve a linear history. The squash commit, I
> > believe, would allow for linear history from this point forward.
> >
> > Github lets repositories be configured to allow committers to choose either
> > option or restrict it to a single option. I am proposing a switch to
> > "squash commit" as the only option on the primary Katello repositories. I
> > would like to get feedback from committers with either a '+1' for agreement
> > or discussion as to the prons/cons of this approach.
> >
> >
> > Thanks
> >
> > –
> > Eric D. Helms
> > Red Hat Engineering
> > Ph.D. Student - North Carolina State University
> >
> > –
> > You received this message because you are subscribed to the Google Groups
> > "foreman-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to foreman-dev+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
> I am against squash commit as the only option. Squashing sometimes makes sense, usually only when there is one person who owns the feature or fix. But more than once I've worked on a branch with other people making commits. In those pull-requests we've been asked to squash the commit down to one which is a bit of a slap in the face to me. Multiple people have contributed to a given feature or fix, but squashing commits only allows for one name attached to the changes, removing authorship and credit from the others who contributed. I certainly don't want my name removed from code I've written and I'd assume a similar sentiment from others?
It's not ideal, but I don't really mind when that happens, if it's for
the sake of having a clean history. You can squash all your changes in
that branch into 2 clean commits that can be cp-ed if you're working
with someone else if you want to preserve authorship.
···
On 04/06, Adam Price wrote:
> ----- Original Message -----
If we’re wanting to use Github’s squash commit feature, I’m apprehensive but I guess it’s alright. What I would prefer is the choice of both because there are occasions where it makes more sense to leave multiple commits in a pull-request.
Just out of curiosity, is Foreman going to be using this instead of
manually cherry picking commits?
David
···
On Tue, Apr 5, 2016 at 3:56 PM, David Davis wrote:
Something to be aware of is that the final commit message is based on the
PR title plus the commit messages of the commits being squashed. You can
however customize the message when you merge.
David
On Tue, Apr 5, 2016 at 3:13 PM, Stephen Benjamin stephen@redhat.com > wrote:
----- Original Message -----
From: “Eric D Helms” ericdhelms@gmail.com
To: “foreman-dev” foreman-dev@googlegroups.com
Sent: Tuesday, April 5, 2016 1:22:02 PM
Subject: [foreman-dev] Katello PRs and merge vs. squash commits
All,
Github has added supported for merging PRs via squash commit in
addition to
the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no "Merge pull request"
commits
in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose
either
option or restrict it to a single option. I am proposing a switch to
"squash commit" as the only option on the primary Katello repositories.
I
would like to get feedback from committers with either a ‘+1’ for
agreement
or discussion as to the prons/cons of this approach.
+1
Yes, definitely. I’d like to see Foreman and Katello adopt the same
commit history
style.
Beyond that, how about no longer manually squash and force pushing PR’s?
Just add
additional commits, as they’ll be squashed by the Big Green Button
Clicker. It will
need changes to the PR processor, because I believe the squash commit
message is
generated from the PR title, so that’s all it should need to check.
IHMO it makes the review process so much easier. If I make some comments
on the PR,
and then the PR author makes those changes as an additional commit, I can
easily
review what changed.
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
> It's not ideal, but I don't really mind when that happens, if it's for
> the sake of having a clean history. You can squash all your changes in
> that branch into 2 clean commits that can be cp-ed if you're working
> with someone else if you want to preserve authorship.
Exactly, I think these are rare cases when you have multiple authors and
rebasing/pushing is matter of few commands while keeping all the
authors.
It has to be the only option, otherwise we are not getting linear
history.
> From: "David Davis" <daviddavis@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Tuesday, April 5, 2016 3:58:33 PM
> Subject: Re: [foreman-dev] Katello PRs and merge vs. squash commits
>
> Just out of curiosity, is Foreman going to be using this instead of
> manually cherry picking commits?
Something to be aware of is that the final commit message is based on the
PR title plus the commit messages of the commits being squashed. You can
however customize the message when you merge.
David
On Tue, Apr 5, 2016 at 3:13 PM, Stephen Benjamin stephen@redhat.com > > wrote:
----- Original Message -----
From: “Eric D Helms” ericdhelms@gmail.com
To: “foreman-dev” foreman-dev@googlegroups.com
Sent: Tuesday, April 5, 2016 1:22:02 PM
Subject: [foreman-dev] Katello PRs and merge vs. squash commits
All,
Github has added supported for merging PRs via squash commit in
addition to
the traditional merge request. This means:
If multiple commits in a merge request, they are squash into 1 (i.e.
fairly standard for acceptance criteria)
Commits are merged a single commit with no “Merge pull request”
commits
in the repository
Katello has historically used the merge pull request button instead of
committing and pushing to achieve a linear history. The squash commit, I
believe, would allow for linear history from this point forward.
Github lets repositories be configured to allow committers to choose
either
option or restrict it to a single option. I am proposing a switch to
“squash commit” as the only option on the primary Katello repositories.
I
would like to get feedback from committers with either a ‘+1’ for
agreement
or discussion as to the prons/cons of this approach.
+1
Yes, definitely. I’d like to see Foreman and Katello adopt the same
commit history
style.
Beyond that, how about no longer manually squash and force pushing PR’s?
Just add
additional commits, as they’ll be squashed by the Big Green Button
Clicker. It will
need changes to the PR processor, because I believe the squash commit
message is
generated from the PR title, so that’s all it should need to check.
IHMO it makes the review process so much easier. If I make some comments
on the PR,
and then the PR author makes those changes as an additional commit, I can
easily
review what changed.
Thanks
–
Eric D. Helms
Red Hat Engineering
Ph.D. Student - North Carolina State University
> Just out of curiosity, is Foreman going to be using this instead of
> manually cherry picking commits?
Speaking for myself, I will most likely carry on with my own script
which also adds comment, thank to the author and close the PR. But there
should be no difference if you use the button or not.
> > Something to be aware of is that the final commit message is based on the
> > PR title plus the commit messages of the commits being squashed. You can
> > however customize the message when you merge.
Yeah, let's put it down to our description on our site that the PR title
is key.
I don't understand why linear history is so important. We have 30+ people working on these projects. The history is going to get a little messy. That's why we use git.
···
----- Original Message -----
> > It's not ideal, but I don't really mind when that happens, if it's for
> > the sake of having a clean history. You can squash all your changes in
> > that branch into 2 clean commits that can be cp-ed if you're working
> > with someone else if you want to preserve authorship.
>
> Exactly, I think these are rare cases when you have multiple authors and
> rebasing/pushing is matter of few commands while keeping all the
> authors.
>
> It has to be the only option, otherwise we are not getting linear
> history.
>
> --
> Later,
> Lukas #lzap Zapletal
>
> From: "Adam Price" <adprice@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Thursday, April 7, 2016 11:03:09 AM
> Subject: Re: [foreman-dev] Katello PRs and merge vs. squash commits
>
> > > It's not ideal, but I don't really mind when that happens, if it's for
> > > the sake of having a clean history. You can squash all your changes in
> > > that branch into 2 clean commits that can be cp-ed if you're working
> > > with someone else if you want to preserve authorship.
> >
> > Exactly, I think these are rare cases when you have multiple authors and
> > rebasing/pushing is matter of few commands while keeping all the
> > authors.
> >
> > It has to be the only option, otherwise we are not getting linear
> > history.
> >
> > –
> > Later,
> > Lukas #lzap Zapletal
> >
>
> I don't understand why linear history is so important. We have 30+ people
> working on these projects. The history is going to get a little messy.
> That's why we use git.
>
We do indeed have a lot of people working together, on a lot of different GitHub
projects across the Foreman and Katello orgs. Why not standardize on 1 way of
doing things when we can?
Personally, I find the linear histories much easier to work with. Katello's history
is horrendously difficult to parse.
···
----- Original Message -----
> ----- Original Message -----
It’s not ideal, but I don’t really mind when that happens, if it’s
for
the sake of having a clean history. You can squash all your changes
in
that branch into 2 clean commits that can be cp-ed if you’re working
with someone else if you want to preserve authorship.
Exactly, I think these are rare cases when you have multiple authors
and
rebasing/pushing is matter of few commands while keeping all the
authors.
It has to be the only option, otherwise we are not getting linear
history.
–
Later,
Lukas #lzap Zapletal
I don’t understand why linear history is so important. We have 30+ people
working on these projects. The history is going to get a little messy.
That’s why we use git.
We do indeed have a lot of people working together, on a lot of different
GitHub
projects across the Foreman and Katello orgs. Why not standardize on 1
way of
doing things when we can?
Personally, I find the linear histories much easier to work with.
Katello’s history
is horrendously difficult to parse.
> From: "David Davis" <daviddavis@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Thursday, April 7, 2016 5:21:09 PM
> Subject: Re: [foreman-dev] Katello PRs and merge vs. squash commits
>
> How are you viewing the history? There’s a bunch of great git log options
> like “–no-merges” that make the git history on katello easier to parse.
>
>
> David
Usually if I'm looking in the history it's to research a change that broke
something. I'm generally doing that in the github web ui, which doesn't
seem to have this no-merges option. I like being able to click on and view
the PR comment history for a commit, or looking at GitHub's Blame UI.
It's a small complaint. But I have a bunch of them about the merge style
history.
If it's not a clear majority preferring one way or the other maybe we should
have a poll.
Stephen
···
----- Original Message -----
On Thu, Apr 7, 2016 at 11:24 AM, Stephen Benjamin stephen@redhat.com > wrote:
It’s not ideal, but I don’t really mind when that happens, if it’s
for
the sake of having a clean history. You can squash all your changes
in
that branch into 2 clean commits that can be cp-ed if you’re working
with someone else if you want to preserve authorship.
Exactly, I think these are rare cases when you have multiple authors
and
rebasing/pushing is matter of few commands while keeping all the
authors.
It has to be the only option, otherwise we are not getting linear
history.
–
Later,
Lukas #lzap Zapletal
I don’t understand why linear history is so important. We have 30+ people
working on these projects. The history is going to get a little messy.
That’s why we use git.
We do indeed have a lot of people working together, on a lot of different
GitHub
projects across the Foreman and Katello orgs. Why not standardize on 1
way of
doing things when we can?
Personally, I find the linear histories much easier to work with.
Katello’s history
is horrendously difficult to parse.
> How are you viewing the history? There’s a bunch of great git log options
> like “–no-merges” that make the git history on katello easier to parse.
This is how it looks like - top is Katello, bottom is Foreman:
I'm viewing it using tig and it is really messy. The main issue I have
with non-linear histories is how difficult it is to go back and branch
from a certain point in time, revert commits, etc… I used to do that a
lot In our kind of model where we always keep pushing forward it's
not critical, granted. If it affected the development of the project,
I'm sure Katello would've moved to a linear history a long time ago!
···
On 04/07, David Davis wrote:
David
On Thu, Apr 7, 2016 at 11:24 AM, Stephen Benjamin stephen@redhat.com > wrote:
It’s not ideal, but I don’t really mind when that happens, if it’s
for
the sake of having a clean history. You can squash all your changes
in
that branch into 2 clean commits that can be cp-ed if you’re working
with someone else if you want to preserve authorship.
Exactly, I think these are rare cases when you have multiple authors
and
rebasing/pushing is matter of few commands while keeping all the
authors.
It has to be the only option, otherwise we are not getting linear
history.
–
Later,
Lukas #lzap Zapletal
I don’t understand why linear history is so important. We have 30+ people
working on these projects. The history is going to get a little messy.
That’s why we use git.
We do indeed have a lot of people working together, on a lot of different
GitHub
projects across the Foreman and Katello orgs. Why not standardize on 1
way of
doing things when we can?
Personally, I find the linear histories much easier to work with.
Katello’s history
is horrendously difficult to parse.
Unless I am mistaken, it seems like only one person has voted against it.
In which case, I vote we turn on the squash-only option and try it out for
a while. I ask only that whoever does this does so for all repos though to
maintain consistency.
David
···
On Thu, Apr 7, 2016 at 5:31 PM, Stephen Benjamin wrote:
How are you viewing the history? There’s a bunch of great git log options
like “–no-merges” that make the git history on katello easier to parse.
David
Usually if I’m looking in the history it’s to research a change that broke
something. I’m generally doing that in the github web ui, which doesn’t
seem to have this no-merges option. I like being able to click on and view
the PR comment history for a commit, or looking at GitHub’s Blame UI.
It’s a small complaint. But I have a bunch of them about the merge style
history.
If it’s not a clear majority preferring one way or the other maybe we
should
have a poll.
Stephen
On Thu, Apr 7, 2016 at 11:24 AM, Stephen Benjamin stephen@redhat.com > > wrote:
To me, that’s a problem with the way you’re viewing your git history and
not the actual git history itself. Not sure about tig but again I think if
you use options with git log like --max-parents, --no-merges, etc, it makes
working with the merge history model far easier:
I’m viewing it using tig and it is really messy. The main issue I have
with non-linear histories is how difficult it is to go back and branch
from a certain point in time, revert commits, etc… I used to do that a
lot In our kind of model where we always keep pushing forward it’s
not critical, granted. If it affected the development of the project,
I’m sure Katello would’ve moved to a linear history a long time ago!
David
On Thu, Apr 7, 2016 at 11:24 AM, Stephen Benjamin stephen@redhat.com > > wrote: