monterail / guidelines

[DEPRECATED] We are Ruby on Rails experts from Poland. Think hussars. Solid & winged. These are our guidelines.
71 stars 17 forks source link

More info on merging next branch to master #205

Closed szajbus closed 9 years ago

szajbus commented 10 years ago

More detailed procedure of merging next to master

Review on Reviewable

sheerun commented 10 years ago

No, we never rebase next.

sheerun commented 10 years ago

An there's no need to create fresh next from master.

szajbus commented 10 years ago

Why not? Both would make the history cleaner.

Especially constantly merging next to master makes the commit tree very messy (in git log --graph)

sheerun commented 10 years ago

Because you never rebase branch that more than one person is working on.

A rebase is only allowed before push. A rebase of next would force every developer to git reset --hard origin/next and cause lot of problems on git pull

szajbus commented 10 years ago

When you're using git pull --rebase (which you should always do anyway) there's no problem.

Like I mentioned, my primary reason is that git log --graph can't handle such history in nice way and every visual tool is based on this command. This make these toos pretty useless or at least very hard to use.

sheerun commented 10 years ago

No, git pull --rebase does not help if remote branch has been rebased.

szajbus commented 10 years ago

Yes, it does :)

sheerun commented 10 years ago

Rebase rewrites history.

http://git-scm.com/book/ch3-6.html

Do not rebase commits that you have pushed to a public repository. If you follow that guideline, you’ll be fine. If you don’t, people will hate you, and you’ll be scorned by friends and family.

What you want to do is to rebase next branch, which is already pushed to public repository.

git pull --rebase rebases local branch, which is the only situation where this is ok.

Besides git merge next will often fast-forward, so history will be clean.

szajbus commented 10 years ago

git pull --rebase rebases local branch on top of remote branch. You do it anyway every time you commit to next. So you even won't notice.

My assumption was that you merge with --no-ff. Just like with feature branches. I now realize it may actually not be necessary.

On the other hand with hotfixes on master you're not going to have a clean history after merge.

sheerun commented 10 years ago

--no-ff always creates merge commit. You probably mean --ff which we don't use.

szajbus commented 10 years ago

I meant --no-ff when merging next to master (after rebase). It looks clean despite the merge commit.

--ff after rebase is clean as well. But --ff without rebase is not clean when there are hotfix commits on master in the meantime.

jandudulski commented 10 years ago

git pull --rebase is a different story than git rebase master done on next. Please don't do this.

szajbus commented 10 years ago

Don't do git pull --rebase? I'd say "always do it".

jandudulski commented 10 years ago

When you do git pull --rebase it would only change your local order of commits- and that's ok.

When you do git rebase master on next you would need to do git push --force to make a push, and you will change the git tree for everyone. That's the difference, and it could make a nice mess.

teamon commented 10 years ago

After hotfix master should be merged into next -> you never have to rebase next on master

teamon commented 10 years ago

/cc @monterail/dev

sheerun commented 10 years ago

@teamon Next should never be rebased anyway, because it's a public branch.

As for feature branches, it's necessary for history to be clean, among other reasons:

a
| \
b d
| /
c
|
e

vs

a
| \
b d
|  |
(...)
| /
y
|
z

This change is not even up to discussion. We never want to:

  1. Rebase next (public branch)
  2. Merge next to master with --ff (lost history, plus not possible without rebase)
sheerun commented 10 years ago

@szajbus I dare you to rebase next on master in any actively developed project.

Ostrzy commented 10 years ago

:+1: For never rebasing next and I generally agree with @sheerun

teamon commented 9 years ago

I'm closing this one.

@Ostrzy @jandudulski @szajbus feel free to reopen if you have any more comments on the topic.