Enable Squash and Merge on every repo and document merge procedure

hug-dev commented 2 years ago

This was discussed on the community meeting, see the related minutes here.

This is about two things:

Activating Squash and Merge as an option on all repos, but not setting it as default.
Adding the merge procedure that the maintainer merging the PR should follow in the book.

MattDavis00 commented 2 years ago

Might be worth mentioning that pull-request templates may help.

For example:

Markdown:

<!---
Please use the PR template to explain why your pull request is needed,
what changes it makes, and how you would like it to be merged.
-->

### What changes does this PR make?:
Details...

### Issue:
Fixes #ISSUE_NUMBER

### Merge Method:
- [X] Merge
- [ ] Squash & Merge
- [ ] Rebase & Merge

Preview:

What changes does this PR make?:

Details...

Issue:

Fixes #ISSUE_NUMBER

Merge Method:

[X] Merge
[ ] Squash & Merge
[ ] Rebase & Merge

hug-dev commented 2 years ago

This is my personnal opinion but in general I am not a fan of templates 😬. Even though they are definitely better for maintainers to understand easily what the PR is about, they add more administrative work on the contributor side to fill the required sections. I think that the general philosophy, at least for now as contributions are low, should be to put all (or most) work on the maintainers' side and as less as possible for the contributor in order to make it as easy as possible for people to contribute. Even people less experienced with Open-source, GitHub workflows, git, etc... This kind of template might frighten new contributors who already made a long way to publish their PR and who now have to fill the template. I would say even if the PR description is not great, the commit messages not perfect, let's accept it how it is and work with the contributor to make is better if needed.

About the Merge Method (which was the original goal for the template, sorry to disgress 🙄) I have the same thinking than above: some new contributors might not even know about those merge method and what to do. I would say that for maintainers, in most cases there are two choices what to do:

The contributor knows about git, cares about its commits and always rebase them and force-push after changes. Example: https://github.com/parallaxsecond/rust-cryptoki/pull/24. In that case, the maintainer should either merge or rebase and merge (better in my opinion if that does not create a merge commit).
The contributor is either new to git, does not care about its commits (which is totally fine in my opinion) or sends a new fix commit for every review step (which is actually quite nice for reviewing). Example: https://github.com/parallaxsecond/parsec/pull/354. In that case the maintainer should ask the contributor if they are ok to squash and merge and then do it.

This is all my personal opinion though!

MattDavis00 commented 2 years ago

Yeah I agree. I don't really mind either way; I kind of have a love hate relationship with templates. On the one hand it makes sure you get all of the information you need in a structured official format. On the other hand I don't like them because it seems verbose and takes extra time. It also disables stuff like auto filling the commit messages in the PR description which is annoying 😢 was just aiming to voice the suggestion mainly

parallaxsecond / parsec-book