Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR merging options: merge commit, rebase and squash #47

Open
bernhardmgruber opened this issue Nov 17, 2020 · 4 comments
Open

PR merging options: merge commit, rebase and squash #47

bernhardmgruber opened this issue Nov 17, 2020 · 4 comments

Comments

@bernhardmgruber
Copy link
Collaborator

bernhardmgruber commented Nov 17, 2020

Previous discussion in: alpaka-group/alpaka#1103

Should we establish a common guideline how PRs are merged? And what should it be?
Currently, alpaka does a rebase and most other projects create a merge commit.

@ax3l
Copy link
Member

ax3l commented Apr 8, 2021

In case it's useful, I use squash commits these days on all repos, since it saves me from explaining to new/external contributors how to rebase :)

@psychocoderHPC
Copy link
Member

What I do not like when using squash commits is that you destroy useful structured PR. If the commit is splitting up the changes into grouped commits it helps when looking later (without github) into the changes.
In cases where the PR is very tiny (hopefully in most cases) it is very useful to squash the commits because nobody will be interested that a five-line change took N commits until the CI passed.

@SimeonEhrig
Copy link
Contributor

I agree with René. A clear commit structure is a part of the documentation. That's something, which I teach my student at the moment.

@bernhardmgruber
Copy link
Collaborator Author

If the commit is splitting up the changes into grouped commits it helps when looking later (without github) into the changes.

Fully agree.

A clear commit structure is a part of the documentation.

Fully agree.

In cases where the PR is very tiny (hopefully in most cases) it is very useful to squash the commits because nobody will be interested that a five-line change took N commits until the CI passed.

Here I see the PR author responsible for rebasing the commit history accordingly. But as Axel pointed out, that requires the author to be fimilar with rebasing. However, being able to rebase is such a versatile technique, I believe people should go the extra mile and acquire the skill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants