Git and Github: best practices for merging branches in my repo

[Wrichik Basu]

At first, let us assume the following: I have a repo named MyRepo that is also on Github (origin). The repo has two branches: master and dev. Both the branches are synchronised with Github.

Now, I want to merge dev with master. For doing this, I have two options:

1. Locally merge dev into master, clear conflicts (if any), and complete the merge. If master has diverged after dev was created, I can rebase master into dev, and then do the merge. Once the merge is complete, I can push master to origin.
2. Rebase (if necessary) and push dev to origin. Then create a pull request on Github, manage conflicts, and merge the pull request.

For a private repo where I am the only contributor, it doesn't matter which option I choose (assuming that I will consistently follow that option in the future). However, for a public repo, or a private repo with more than one collaborator, I should follow the best practice. Which of the above options is best for merging repos that are on Github?

 

[pbuk]

However, for a public repo, or a private repo with more than one collaborator, I should follow the best practice. Which of the above options is best for merging repos that are on Github?

On a collaborative project use a pull request. Even when I am the only one working on a project I find the discipline of a PR useful. What workflow are you using that gets main ahead of develop?

 

[PeterDonis]

If master has diverged after dev was created, I can rebase master into dev, and then do the merge.

I assume you mean rebase dev so it's branched from the new master, meaning that the merge of dev into master will be a fast-forward merge.

Rebasing means rewriting history; you should never rewrite the history of master. That's the most important special case of the general rule that you should never rewrite history that has been made publicly visible (since doing that screws up everyone else's copies of the repo).

 

[Wrichik Basu]

What workflow are you using that gets main ahead of develop?

I believe you ask this question because I mentioned rebasing in my OP. To be honest, I am an absolute beginner in Git. I do not yet have a public repository, but have a few private ones that I will make public (open source) after I get an idea of how Github works. Having a look at the different kinds of distributed workflows, I like the Integration Manager workflow the best.

To answer your question as to how the master can go ahead of dev, consider the following: while I am working on dev, someone else creates a pull request to master on Github. I pull that person's repo locally, find that it is fine, and hence merge the PR on Github. In this situation, master will go ahead of dev. Is this situation possible?

If yes, I have a follow-up question: suppose my master has diverged from dev. Before I merge dev into master, should I rebase it so as to maintain a linear history, or should I merge master into dev first (locally) and then push it to Github, create a PR and merge with master?

 

[Wrichik Basu]

I assume you mean rebase dev so it's branched from the new master, meaning that the merge of dev into master will be a fast-forward merge.

Rebasing means rewriting history; you should never rewrite the history of master. That's the most important special case of the general rule that you should never rewrite history that has been made publicly visible (since doing that screws up everyone else's copies of the repo).

Yes, I meant to say I would rebase dev. Sorry for the confusion, if any. Please see my post above.

 

[PeterDonis]

Before I merge dev into master, should I rebase it so as to maintain a linear history, or should I merge master into dev first (locally) and then push it to Github, create a PR and merge with master?

The question of rebasing is actually separate from the question of whether you should merge locally or push the branch to Github and create a PR.

If you rebase dev, you can still push it to Github after the rebase and create a PR instead of merging the rebased dev into master locally and then just pushing the new master to Github. (Of course this assumes that dev is purely a local branch in your local repo and doesn't exist on Github; if it is publicly visible you should never rebase it, according to the rule I stated before.)

If you merge master into dev locally to capture the changes in master in your local dev branch, you can still merge dev back into master locally instead of pushing dev to Github and creating a PR.

People's opinions differ on rebasing; some people dislike rewriting history at all, others dislike having a "messy" history that has false starts and rewrites in it instead of the cleaner history you can get if you rebase (and also rewrite history on local branches in other ways, such as squashing commits).

As far as PRs, I think it comes down to whether you want the changes in your dev branch to be publicly recorded in a PR, which can have not just the commits but also discussion surrounding them, or to just be visible as one or more commits, with no other comments or discussion besides whatever is in the commit log. That might depend on project custom, and also on how complex the changes are.

 

[pbuk]

I believe you ask this question because I mentioned rebasing in my OP. To be honest, I am an absolute beginner in Git. I do not yet have a public repository, but have a few private ones that I will make public (open source) after I get an idea of how Github works. Having a look at the different kinds of distributed workflows, I like the Integration Manager workflow the best.

To answer your question as to how the master can go ahead of dev, consider the following: while I am working on dev, someone else creates a pull request to master on Github. I pull that person's repo locally, find that it is fine, and hence merge the PR on Github. In this situation, master will go ahead of dev. Is this situation possible?

Not really. I think the Git documentation is rather confusing about workflows. When most people talk about workflows they are talking about a branching strategy rather than collaboration between separate repos.

If you have a dev(elop) branch then you are implicitly using a form of the GitFlow workflow. I think the Atlassian tutorial explains this better than the Git documentation, and as you can see this does not include the possibility of a collaborator issuing a feature PR to main (or master), features are always merged into develop.

Also note that GitHub is changing its default branch to main and it makes sense to use this instead of master for new projects (and to migrate existing live projects when feasible).

If yes, I have a follow-up question: suppose my master has diverged from dev. Before I merge dev into master, should I rebase it so as to maintain a linear history, or should I merge master into dev first (locally) and then push it to Github, create a PR and merge with master?

@PeterDonis has already covered this but just to repeat, you should never rebase anything in a shared repo, but it is OK to rebase say your feature-add-exception-handling branch against develop before issuing a PR. The feature branch should then be deleted.

 

[Wrichik Basu]

The question of rebasing is actually separate from the question of whether you should merge locally or push the branch to Github and create a PR.

If you rebase dev, you can still push it to Github after the rebase and create a PR instead of merging the rebased dev into master locally and then just pushing the new master to Github. (Of course this assumes that dev is purely a local branch in your local repo and doesn't exist on Github; if it is publicly visible you should never rebase it, according to the rule I stated before.)

If you merge master into dev locally to capture the changes in master in your local dev branch, you can still merge dev back into master locally instead of pushing dev to Github and creating a PR.

People's opinions differ on rebasing; some people dislike rewriting history at all, others dislike having a "messy" history that has false starts and rewrites in it instead of the cleaner history you can get if you rebase (and also rewrite history on local branches in other ways, such as squashing commits).

As far as PRs, I think it comes down to whether you want the changes in your dev branch to be publicly recorded in a PR, which can have not just the commits but also discussion surrounding them, or to just be visible as one or more commits, with no other comments or discussion besides whatever is in the commit log. That might depend on project custom, and also on how complex the changes are.

Thanks for the explanation.

I have already rebased master once in order to correct a commit message, but my repo was (and is still) private, so I believe no one will be adversely affected by the rebase.

I plan to keep dev on Github so that people can fork out of it, therefore I will not rebase it. Following the tutorial posted by @pbuk in post #7, I will allow only hotfix branches from master. The dev branch, when appropriate, will be merged with master, and I will create a release from master after that.

Github allows https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges for pull requests: normal merge commit, squash commit and rebase commit.

Two questions arise:

1. For a PR created by someone else, I am not interested in past commits made by that person; I am only interested whether their branch in the current version works properly when merged with master. So I will do either a squash commit or a normal merge commit; rebase commit is out of the question. If I do a normal merge commit, the past commits of that person will be visible in the history of master. In that case, will it be appropriate to do a squash commit? Can I face any problem in the future?​

2. For a PR created by me for merging dev into master, I won't do a squash commit because Github https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squashing-and-merging-a-long-running-branch that squash commit should not be used if I plan to keep working on the HEAD branch. Therefore, I will either do a rebase commit or a normal merge commit. Which is more suitable?​

 

[PeterDonis]

Following the tutorial posted by @pbuk in post #7

Yes, that seems like a good general set of policies.

For a PR created by someone else, I am not interested in past commits made by that person

In some cases, you might be. Sometimes there is valuable information in those individual commits. See below.

I will do either a squash commit or a normal merge commit; rebase commit is out of the question. If I do a normal merge commit, the past commits of that person will be visible in the history of master. In that case, will it be appropriate to do a squash commit? Can I face any problem in the future?

Yes: the change might be incomprehensible to future viewers (including future you) because you squashed it all into one commit. Many changes are not best captured that way; there are often multiple conceptually separate things that are being done, in some logical order, and squashing throws away all that information.

What I have done as a person submitting pull requests in the past is to try to organize the pull request in such a way that the individual commits in it will each convey useful information, so that it will be useful to merge them as-is instead of squashing them. Sometimes that means reorganizing my own internal work on my own internal branch into a different set of commits than the ones I originally made when I was still figuring out what to do. I, the person who did that work, am going to understand how best to do that better than anyone else, so it makes sense for me to do it, rather than just throwing a haphazard set of commits at the project owner and hoping they will figure something out.

I would encourage you, as a project manager, to adopt a policy like that towards people who submit pull requests; if the PR doesn't look like it can be merged as-is with a useful set of commits that properly capture what changes were made and why, request that the submitter reorganize it so it can be. If it turns out that one commit does the job, fine--but the burden should be on the submitter to figure that out, not you having to decide whether to squash or not.

For a PR created by me for merging dev into master, I won't do a squash commit because Github https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squashing-and-merging-a-long-running-branch that squash commit should not be used if I plan to keep working on the HEAD branch. Therefore, I will either do a rebase commit or a normal merge commit. Which is more suitable?

In this case, you are the PR submitter, so I think you should do what I suggested above that every PR submitter should do.

 

[pbuk]

I have already rebased master once in order to correct a commit message, but my repo was (and is still) private, so I believe no one will be adversely affected by the rebase.

Do you mean private or do you mean purely local? ISTR that both GitHub and Bitbucket will object to pushing a rebased HEAD to an existing remote: the workaround is to delete the remote repo and recreate it from your master copy.

1. For a PR created by someone else, I am not interested in past commits made by that person; I am only interested whether their branch in the current version works properly when merged with master. So I will do either a squash commit or a normal merge commit; rebase commit is out of the question. If I do a normal merge commit, the past commits of that person will be visible in the history of master. In that case, will it be appropriate to do a squash commit? Can I face any problem in the future?​

I've got a slightly different take to @PeterDonis on this one, but accept that his is just as correct. Firstly, when you start a PR on GitHub it will create an (editable) comment listing all the commit comments. Also, most projects enforce a 'one feature one PR' rule. Taking these together, the only thing that not --squashing will give you is the state of the repo at every commit in developing the PR - which I have never needed.

You will also notice that GitHub offers to delete the branch after a successful PR, which you will want to do after you have --squashed it into develop.

2. For a PR created by me for merging dev into master, I won't do a squash commit because Github https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squashing-and-merging-a-long-running-branch that squash commit should not be used if I plan to keep working on the HEAD branch. Therefore, I will either do a rebase commit or a normal merge commit. Which is more suitable?​

A normal (fast-forward) commit. You should never rebase your HEAD branch.

 

[Wrichik Basu]

Do you mean private or do you mean purely local? ISTR that both GitHub and Bitbucket will object to pushing a rebased HEAD to an existing remote: the workaround is to delete the remote repo and recreate it from your master copy.

Actually I did the following: I rebased the local repo to change two erroneous commit messages. Once I force pushed the commits, I found that a PR merge was missing from the history. So I decided to ditch the remote repo and created a new repo, and pushed the local to that remote. All this was done in a private remote repo. You won't be able to understand I did this unless you look at reflog.

 

[PeterDonis]

most projects enforce a 'one feature one PR' rule.

Even with this rule (which I agree is a good one), "one feature" can still be enough of a change to warrant being captured in more than one commit. This is probably more likely to be true on a large and complex project, not a small one with only a few developers, but I think it's worth keeping in mind.

 

[Wrichik Basu]

I wrote the following file, and this will be approximately the same across all my apps. Let me know if I should add/subtract something:

# Welcome to <app-name>

Issues and suggestions for improvements are always welcome. Anyone is allowed to fork this repository, make changes and submit a pull request (PR). I will be happy to make changes to my app for making it better.

However, before contributing, please read the following guidelines:

## Guidelines for contributing:
1. Please fork from the `dev` branch and submit PRs to the `dev` branch. PRs directly to the `master` branch will not be entertained.
1. Exception to the above rule: only `hotfix` branches are allowed from the `master`. These branches aim at squashing bugs that severely impact the basic functionality of the app. Once approved, `hotfix` branches will be merged with master as well as `dev`.
1. Do not change the following:
   - Gradle version
   - Android SDK version
   - Build tools version
   - Version of any other dependency, even if it is outdated
   - App version and/or version codes
1. Please try to make a logical series of commits in your PR such that they have meaning when someone looks at the history of the project. If this is not followed, I will do a squash merge rather than normal merge (doing so will put all your commits into one single commit in the history of your project).
1. Clear merge conflicts, if any, that Github shows when you submit the PR. I will help if you need.
1. Follow the "one feature one PR" rule: each PR should add only one feature to the app. You can change as many files as you need for introducing the feature, but these changes should cater to only one feature.
1. Document your code where necessary. Changes without proper documentation may not be accepted.

 

[pbuk]

Unit + integration + functional tests?

 

[Wrichik Basu]

Unit + integration + functional tests?

My apps are generally of only one module, and I test them by directly running the whole app on an emulator or real device. Writing integration, unit and functional tests is quite time-consuming, and I find a combination of logging + debug run much easier and time-saving.