dbt-labs / docs.getdbt.com

The code behind docs.getdbt.com
https://docs.getdbt.com/
Apache License 2.0
118 stars 928 forks source link

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

Open dbeatty10 opened 3 months ago

dbeatty10 commented 3 months ago

Problem

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

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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

dbeatty10 commented 2 months ago

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 commented 2 months ago

@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.