Squashing, Git, PRs, and Maybe Other Processes

[Wade Chandler <wa...@apache.org>]

All

We are using PRs and Git. I have not seen discussions of the git flow we
might use beyond PRs; maybe I missed it.

My teams and I use PRs, and that is the atomic operation for merging code.
We do not squash because it obscures the path of how the logic came into
being from our point of view.

I know some like squashing because it some how makes logs "more clean", but
I don't see it this way as a group of related changes may have some
specific info related to subsets of those changes, and thus the PR is the
atomic item merged to master at once, and not the cohesive set of
iindividual commits.

On rebasing, when working on a branch, I think that is useful. This does
not remove anyone's messages and log detail, but does keep out an
unnecessary log message when bringing down non-conflicting changes.

I also think it is crucial to keep feature branches updated with master.
One wants the merge pain on the branch, and not when going to master. Then,
one should be able to validate their changes before going to the master
build.

I have seen some Apache processes which suggest contradictions to the above:

https://cwiki.apache.org/confluence/display/JCLOUDS/Git+workflow

But, in practice over many years, the above has worked well in highly
collaborative, high traffic, and high profile environments; I do not want
to find the issues on master.

Next, are we able to merge PRs at the Github side? That is more straight
forward in practice to me. The idea being before that is done, one has
tested and validated locally or on branch ahead of time, and reduces
further manual touches of the merge. For instance, Github, like Bitbucket,
won't accidentally force a merge. The conflicts must first be resolved.

How do you all feel about these issues?

Thanks

Wade

Re: Squashing, Git, PRs, and Maybe Other Processes

[Wade Chandler <wa...@apache.org>]

Thanks Emmanuel, comments inline...

On Oct 11, 2017, at 12:11, ehsavoie <em...@gmail.com> wrote:
> 
> Hi,
> About squashing : having a single commit makes it easier to
> - backport for maintenance
> - revert

Agree to a point there; it is definitely “easier”, having a single value, but cherry picking a range is pretty trivial as well:
https://www.tollmanz.com/git-cherry-pick-range/ <https://www.tollmanz.com/git-cherry-pick-range/>

Revert and other commands can work with a range too, and getting the range from a point in time relative to a specific merge is pretty easy (especially if there is a PR). The UI for a PR will let one click revert and do the same thing if using GH or Bitbucket as an example.

> - review : you don't review some code that might disappear in the next
> commit

I’m not sure I follow here; that seems always like a possibility if it is one.

> Maybe the task should be divided into multiple sub task thus allowing
> multiple PRs

I agree that is always something someone should review and sub-task, but isn’t the way that will always work out.

Thanks,

Wade

Re: Squashing, Git, PRs, and Maybe Other Processes

[ehsavoie <em...@gmail.com>]

Hi,
About squashing : having a single commit makes it easier to
 - backport for maintenance
 - revert
 - review : you don't review some code that might disappear in the next
commit
Maybe the task should be divided into multiple sub task thus allowing
multiple PRs
My 2 cents
Emmanuel

----------
Emmanuel Hugonnet
http://www.ehsavoie.com
http://twitter.com/ehsavoie

On Wed, Oct 11, 2017 at 4:47 PM, Wade Chandler <wa...@apache.org>
wrote:

> All
>
> We are using PRs and Git. I have not seen discussions of the git flow we
> might use beyond PRs; maybe I missed it.
>
> My teams and I use PRs, and that is the atomic operation for merging code.
> We do not squash because it obscures the path of how the logic came into
> being from our point of view.
>
> I know some like squashing because it some how makes logs "more clean", but
> I don't see it this way as a group of related changes may have some
> specific info related to subsets of those changes, and thus the PR is the
> atomic item merged to master at once, and not the cohesive set of
> iindividual commits.
>
> On rebasing, when working on a branch, I think that is useful. This does
> not remove anyone's messages and log detail, but does keep out an
> unnecessary log message when bringing down non-conflicting changes.
>
> I also think it is crucial to keep feature branches updated with master.
> One wants the merge pain on the branch, and not when going to master. Then,
> one should be able to validate their changes before going to the master
> build.
>
> I have seen some Apache processes which suggest contradictions to the
> above:
>
> https://cwiki.apache.org/confluence/display/JCLOUDS/Git+workflow
>
> But, in practice over many years, the above has worked well in highly
> collaborative, high traffic, and high profile environments; I do not want
> to find the issues on master.
>
> Next, are we able to merge PRs at the Github side? That is more straight
> forward in practice to me. The idea being before that is done, one has
> tested and validated locally or on branch ahead of time, and reduces
> further manual touches of the merge. For instance, Github, like Bitbucket,
> won't accidentally force a merge. The conflicts must first be resolved.
>
> How do you all feel about these issues?
>
> Thanks
>
> Wade
>

Re: Squashing, Git, PRs, and Maybe Other Processes

[Wade Chandler <wa...@apache.org>]

Fair enough indeed. It would be good if someone from infra or a mentor
could tell us if PR merges work well enough from the Github side, unless
you all have already been merging by clicking merge on them there, or if
those merges have to be done through the ASF repositories and locally.
Since I am working on the static site, I have not looked at the other PRs
and repositories to see, but it would be great if we are able to use a flow
driven from the PRs directly through GH IMO.

Thanks much,

Wade

On Oct 11, 2017 2:18 PM, "Emilian Bold" <em...@gmail.com> wrote:

I only like squashing because I'm merging single commits and I don't
want another 'merge' commit.

Sure, if a feature is implemented and it has 20 commits, it does have
some importance to preserve those since, if they are well made, there
is some logical progression there that you will need for future
reference.

Other than this aspect, I think we are doing fine. Our changes so far
are minor. Once we start contributing fixes and features and alter
things deeper then we have to think about how to thread on thin ice
more efficiently.


...snip

Re: Squashing, Git, PRs, and Maybe Other Processes

[Emilian Bold <em...@gmail.com>]

I only like squashing because I'm merging single commits and I don't
want another 'merge' commit.

Sure, if a feature is implemented and it has 20 commits, it does have
some importance to preserve those since, if they are well made, there
is some logical progression there that you will need for future
reference.

Other than this aspect, I think we are doing fine. Our changes so far
are minor. Once we start contributing fixes and features and alter
things deeper then we have to think about how to thread on thin ice
more efficiently.

--emi


On Wed, Oct 11, 2017 at 5:47 PM, Wade Chandler <wa...@apache.org> wrote:
> All
>
> We are using PRs and Git. I have not seen discussions of the git flow we
> might use beyond PRs; maybe I missed it.
>
> My teams and I use PRs, and that is the atomic operation for merging code.
> We do not squash because it obscures the path of how the logic came into
> being from our point of view.
>
> I know some like squashing because it some how makes logs "more clean", but
> I don't see it this way as a group of related changes may have some
> specific info related to subsets of those changes, and thus the PR is the
> atomic item merged to master at once, and not the cohesive set of
> iindividual commits.
>
> On rebasing, when working on a branch, I think that is useful. This does
> not remove anyone's messages and log detail, but does keep out an
> unnecessary log message when bringing down non-conflicting changes.
>
> I also think it is crucial to keep feature branches updated with master.
> One wants the merge pain on the branch, and not when going to master. Then,
> one should be able to validate their changes before going to the master
> build.
>
> I have seen some Apache processes which suggest contradictions to the above:
>
> https://cwiki.apache.org/confluence/display/JCLOUDS/Git+workflow
>
> But, in practice over many years, the above has worked well in highly
> collaborative, high traffic, and high profile environments; I do not want
> to find the issues on master.
>
> Next, are we able to merge PRs at the Github side? That is more straight
> forward in practice to me. The idea being before that is done, one has
> tested and validated locally or on branch ahead of time, and reduces
> further manual touches of the merge. For instance, Github, like Bitbucket,
> won't accidentally force a merge. The conflicts must first be resolved.
>
> How do you all feel about these issues?
>
> Thanks
>
> Wade