Commit history and merging Pull Requests

[briederer]

We also need to come up with a clean way of pushing pull requests to master to keep the history clean. Seems like that two (of three) options on the Github-App doesn't work as I expect them to 🤔 Will try the last one and otherwise I won't merge anymore in the app.

[fzierler]

I am not fully sure I understand the problem you are experiencing. We could try to rebase and squash commits before merging any pull requests so that every pull request is linked to a single commit in the correct chronological order. I am using GitKraken which provides a nice GUI for this.

[briederer]

Yes I think too that rebase/squash is the way to go (also since with rebase one could combine multiple PRs if needed). Also this is no problem to do at the PC but the GitHub-Smartphone App provides only three possibilities to merge:

• Simple merge
• Squash and merge
• Rebase and merge

So far I've tried the first (CompatHelper PR #1) and the third (for #4) and the merges were a bit messy. So basically I just want to figure out if there is a possibility to have nice merges using the Smartphone app 😉

[fzierler]

Ah, I did not think about the mobile app. I could still squash the latest three commits of #4 on master although I see why it is preferable to do this before merging.

Edit: Also I think we should try to continue not merging our own pull requests. This should make for a good code review workflow.

[briederer]

Re squashing: this would be nice of you thanks.

Re self-merging: makes sense. If there is something very urgent to be added immediately we can just push to master. Otherwise file a PR and wait for the other(s) to approve and merge.

[fzierler]

Yes, I don't think self-merging is bad in itself but we can try and avoid it where possible. Also I have now squashed the last commits

[fzierler]

When merging #7 through the Website I encountered the same issue that you had. After merging the pull request both your commit and a merge commit showed up in the history. I am pretty sure that there is a better way of doing this than manually squashing the commits afterwards.

Edit. I had the same issue as you when trying to merge multiple CompatHelper pull requests. It seems the proper way of doing this is to first resolve the conflicts and then to squash&merge.

[briederer]

The following ways seem to be the best:

• Merging one PR (with/out multiple commits):
  • Website/Mobile app: Squash and merge
  • CLI/GUI: Squash and merge
• Merging multiple PRs at once:
  • Website/Mobile app: Don't do it
  • CLI/GUI: Follow this guide and squash/merge the new branch with all PRs to master

[briederer]

I've cleaned up the full commit history now following this (guide)[https://blog.nona.digital/cleaning-up-commit-history-with-git-rebase/amp/] and tried to get some structure into our commit-naming scheme.

My suggestion would be to stick to the following conventions for commits:

• Add: or Fix: for direct pushes to master
  • optional: more infos listed with + and * markers
• PR: for usual Pull requests that are merged
  • PR reference in parantheses
  • squashed commits listed with + marker
• CompatHelper: Added new compat entry/entries for merges from CompatHelper pull requests
  • All PR references in parantheses
  • State changed Project.toml
  • Added Packages listed at * Package 'Packagename' v'versionnumber'

[fzierler]

Great. Let's keep this issue open for now until I manage to submit commits/PR using this convention. ;) It seems that the best way to deal with multiple ComaptHelper PRs is to create a combined one from scratch. This is probably more straightforward than merging multiple PRs and then squashing them.