discuss optimal merge strategies and PR guidelines

[ozanstats]

It could be beneficial to discuss different merge strategies as well as expectations from contributors regarding commits and PRs. Having every single commit in git history might create clutter or squashing too much can make it hard to find important details. What would be your ideal strategy?

[paul-buerkner]

Good point, I must admit that I am out of depth here as I haven't thought of this really until you brought it up. So I would appreciate some comments of people more experienced with the different merging strategies.

@Ozan147 What is your personal opinion on this?

[ozanstats]

I don't have much experience either but here's my two cents.

I personally like the squash and merge strategy, which condenses PR's commits into a single commit on the main branch. I believe it's a nice way to organize the git history if PRs are kept specific and feature branches are cleaned properly. Otherwise, some info could get lost or conflicts may come up, which would take extra effort to resolve. So, maybe rather than changing the default merge strategy, contributors can be encouraged to squash their trivial commits in their feature branches.

Additionally, I personally like to standardize things as much as possible using pre-commit hooks and PR templates in my repos. Pre-commit hooks take care of basic checks like styling and linting. Guiding contributors with a default PR template sets clear expectations. An example would be something like this:

---

Issue

Closes/Fixes/Addresses #

Contribution

...

Checklist

• Pre-merge
  1. [ ] If possible, trivial commits are squashed
  2. [ ] Pre-commit hooks passes (if first-time user, enable via precommit::use_precommit())
  3. [ ] Code conforms to the tidyverse style guide
  4. [ ] Test cases added if necessary
• Post-merge
  1. [ ] Merge completed and branch deleted

[mjskay]

I like the idea of code linting and a PR template.

I'll push back a bit on spending time cleaning up commit history: I'm not sure I see the point of it. Is there a task that people perform that requires looking back at git history that is improved by cleaning it up in this way? I can't think of one, so I find the added time cost of cleaning up history to not be worth the benefit. Plus, not squashing means short commit messages become meaningful comments if you are using git blame / the "blame" view on github.

However I'd be open to being convinced otherwise.

[jgabry]

I like the idea of code linting and a PR template.

Yeah me too.

I'll push back a bit on spending time cleaning up commit history: I'm not sure I see the point of it. Is there a task that people perform that requires looking back at git history that is improved by cleaning it up in this way? I can't think of one, so I find the added time cost of cleaning up history to not be worth the benefit.

I think I agree with this.

However I'd be open to being convinced otherwise.

Same.

[rok-cesnovar]

In other stan-dev repos we mostly do merge commits without squashing. I typically squash only PRs where we had to do a ton of debugging why tests fail and such. I prefer having the entire history personally.