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

Only allow squash commits and disallow merge commits and rebase merge #5635

Open
dbeatty10 opened this issue Jun 10, 2024 · 7 comments
Open

Comments

@dbeatty10
Copy link
Contributor

Problem

The commit history for some source code files is not easy to read due to:

  • more commits than associated pull requests
  • commit messages that are not very descriptive

It looks to me that these PRs were merged with a merge commit rather than a squash merge.

Here's an example:
https://github.com/dbt-labs/docs.getdbt.com/commits/current/website/docs/reference/global-configs/project-flags.md

Proposed solution

Other repos like dbt-core don't allow merge commits and only allow squash merges.

To do the same in docs.getdbt.com, open the repo settings and change this:

image

To this:

image
@runleonarun
Copy link
Collaborator

runleonarun commented Jun 10, 2024

Hey @dbeatty10 it appears this has been done. Was that you who made the change?

I'm just checking with the front end team as we have other things going on and want to make sure this aligns across the board (with front end eng etc)

@runleonarun
Copy link
Collaborator

runleonarun commented Jun 10, 2024

One thing to consider is these disadvantages of merge & squashing:

Before enabling squashing commits, consider these disadvantages:

  • You lose information about when specific changes were originally made and who authored the squashed commits.
  • If you continue working on the head branch of a pull request after squashing and merging, and then create a new pull request between the same branches, commits that you previously squashed and merged will be listed in the new pull request. You may also have conflicts that you have to repeatedly resolve in each successive pull request. For more information, see "About pull request merges."
  • Some Git commands that use the "SHA" or "hash" ID may be harder to use since the SHA ID for the original commits is lost. For example, using git rerere may not be as effective.

@runleonarun
Copy link
Collaborator

runleonarun commented Jun 10, 2024

Note that Jason K likes the idea of having more than one enabled. Though this could be different with an OS repo.

Also I heavily use the commit history to pinpoint changes.

@mirnawong1 @matthewshaver or @nghi-ly any thoughts?

@dbeatty10
Copy link
Contributor Author

Hey @dbeatty10 it appears this has been done. Was that you who made the change?

Oops! I ticked them just for the purpose of a screenshot, and I didn't realize that it saved it automatically! 😬 Sorry about that!

Just now changed it back to what it was originally:

image

@mirnawong1
Copy link
Contributor

@dbeatty10 and @runleonarun do we still need this issue? it seems like we have it squashed now right?

@dbeatty10
Copy link
Contributor Author

All (3) options are still enabled for the docs.getdbt.com repo, whereas "squash merging" is the only option for the dbt Labs repos I use most often:

image

@runleonarun
Copy link
Collaborator

@mirnawong1 I think we just need to talk about this as a team to discuss the tradeoffs. Though, it seems like the default behavior (Squash and merge) is what's mostly used.

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

3 participants