public
public copied to clipboard
socialincome-san
[DevOps Feature]: RFC: Proper contribution guidelines
We need some proper contribution guidelines, especially for the commit messages. I feel like it is necessary to have clear scoping in monorepos to be able to tell in which package the change happens. This allows easier change tracking and also, if wanted, to generate changelogs for releases for individual packages (even though the app is currently the only place where release notes apply).
Conventional commits as often used in many projects, is an approach I like. With that, we could clearly mark the scope in the commit messages between backend, recipients_app, admin, shared, ui and website.
Request for comment @socialincome-san/admins-public-repo
Good suggestion, I'm in.
I put the squash merge as default for prs. So the individual commits get merged into 1 commit, which then maybe can follow this convention?
@andrashe
I put the squash merge as default for prs. So the individual commits get merged into 1 commit, which then maybe can follow this convention?
I usually discourage of doing so because squash merges can destroy a lot of information. I often do many small granular commits and I find it helpful others do the same as the git blame and history reading is much easier and also the individual commits can be cherry picked easier for hotfixes.
I would recommend not making this the default, but an option to choose from to ensure we don't make it standard to have huge, hard to understand commits in the history.
Edit: But to really answer the question, yes, the squash would need to follow the same rule.
I usually discourage of doing so because squash merges can destroy a lot of information. I often do many small granular commits and I find it helpful others do the same as the git blame and history reading is much easier and also the individual commits can be cherry picked easier for hotfixes.
I guess one could argue that then the scope of the PR is too big if you want to do partial cherry-picks of individual commits for hotfixes.
I would recommend not making this the default, but an option to choose from to ensure we don't make it standard to have huge, hard to understand commits in the history.
My fear is that if it is not the default, the main branch is very quickly quite messy because one forgets to squash it.
But also no strong feelings about this for this project. I can enable the normal merge again.
I guess one could argue that then the scope of the PR is too big if you want to do partial cherry-picks of individual commits for hotfixes.
Probably not something that is that relevant to us, sure. That would be more for scenarios like, let's say, you already have a patched a bug in one of your next releases, but the next release it not ready to ship, yet you want already ship that particular patch as a hotfix, so you can use that single patch commit and put it on main. I at least use that possibility quite often in projects where I ship multiple versions (e.g. v1.x.x and v2.x.x, and I need to apply the patch on both versions as they are not backwards compatible).
I had some time ago strong opinion that squash is nice. But... No more. Git history should be for us to make sure that we can get all information we need.
After moving repos to one mono-repo we lost most of the history anyway, but I wouldn't make the "clean commit history" the goal.
I believe everyone should think about commit messages. And if we make some not very useful commit messages (e.g. "Fix typo") we should probably squash it with previous commit before making PR.
I would prefer clear commit message convention with the issue number if possible. It is easier with Jira / Trello to make those ticket identifiers even more human friendly but github issues numbers should be good enough as well with the scopes identifiers as suggested in the discussion.
