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

Squash and merge a PR #124

Open
hpatro opened this issue Sep 5, 2024 · 2 comments
Open

Squash and merge a PR #124

hpatro opened this issue Sep 5, 2024 · 2 comments

Comments

@hpatro
Copy link
Contributor

hpatro commented Sep 5, 2024

Observed with my recent PR (#114) that we merge all the individual commits as part of merging a PR. This is different from github.com/valkey-io/valkey.

Current view of main:

> git log --pretty=format:"%h%x09%an%x09%ad%x09%s"
18470ff Kyle J. Davis   Wed Sep 4 20:09:03 2024 +0200   Merge pull request #119 from madolson/better-format-2
bbc56d3 Kyle J. Davis   Wed Sep 4 20:02:39 2024 +0200   Merge pull request #114 from hpatro/memory_efficiency_blog
641bd9a Madelyn Olson   Wed Sep 4 08:45:38 2024 -0700   Merge branch 'main' into better-format-2
5df57d9 Madelyn Olson   Wed Sep 4 08:40:38 2024 -0700   Revert next alignment for followup
84466f2 Harkrishn Patro Wed Sep 4 07:05:35 2024 -0700   Address feedback
5187955 Kyle J. Davis   Wed Sep 4 08:32:53 2024 +0200   Merge pull request #121 from stockholmux/banner
7a8b213 Madelyn Olson   Tue Sep 3 16:27:13 2024 -0700   new lines, that is a hill I'll die on
9ef3d3d Madelyn Olson   Tue Sep 3 16:24:33 2024 -0700   Revert back to 80 character width
8374db6 Madelyn Olson   Tue Sep 3 16:20:37 2024 -0700   Address some comments
6e1601a Harkrishn Patro Tue Sep 3 16:12:03 2024 -0700   Address feedback
d4fa40a Harkrishn Patro Tue Sep 3 16:07:35 2024 -0700   Rearrange rehashing operation summary
6d45b99 Harkrishn Patro Tue Sep 3 14:32:56 2024 -0700   Apply suggestions from code review
5cae216 Harkrishn Patro Tue Sep 3 11:33:01 2024 -0700   correct y-axis of graph
5e1b621 Kyle J. Davis   Tue Sep 3 15:35:13 2024 +0200   Adds the ability to add a top banner to every page as well as a Valkey Developer Day message
5c1c4dd Madelyn Olson   Sat Aug 31 18:28:10 2024 -0700  Some suggestion for improving the blog
6a5f38c Harkrishn Patro Fri Aug 30 14:01:53 2024 -0700  Update author photo and interest
...

I would suggest to do a squash and merge once the PR is ready. Having a single commit makes life easier in the following ways:

  • Track down bug/issues to a particular commit.
  • Git history is simpler.
  • Revert a single commit to undo the behavior.
  • Allows dev to push smaller commits to their branch (personally prefer this).
@hpatro hpatro added the bug Something isn't working label Sep 5, 2024
@hpatro
Copy link
Contributor Author

hpatro commented Sep 5, 2024

@stockholmux WDYT ?

Also, it's not a bug but I can't remove labels in this repository.

@zuiderkwast zuiderkwast removed the bug Something isn't working label Sep 12, 2024
@stockholmux
Copy link
Member

@hpatro IIRC we tried this but it doesn't work particularly well with how we deploy: prod_zola is the deployment and we do do a PR from main to prod_zola (which triggers the build). If this is a squash and merge it creates this katamari of commits every time that is really hard to resolve.

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