Recently I have been scavenging(xd) older PRs for some help related to an issue. I found it very difficult to understand or for that matter read the commit messages and understand what each commit did until I saw the changes in the file changes section of that commit(PR).
@4ian, I don’t want to compare GDevelop to other organizations but I found Zulip’s concept of contribution guidelines to be very helpful. you may have a look here. At first, it was irritating and tedious to remember each of the commit rules but after I spent time working on some issues I realized why commit discipline is so important and helpful to the whole community.
I am not suggesting you impose the same rule but from your experience as a Software Developer, I am sure that you can impose similar “guidelines” for a commit message. It will not only help you (Since your repository will have cleaner commit messages & PRs) but also the novice contributors like me who want to learn how contributions are done to a codebase as a Software Developer.
I’ve read the Zulip guide and it’s I think a very good guide. I don’t put any written rules on commits because I consider these to be good practice that you naturally acquire when working on software. Almost all PRs are also “squashed” into a single commit when I merge them, allowing me to have a PR that is autonomous and with a proper description.
So I encourage you to follow these rules - again from what I read they seem to be very good.
If later GDevelop is growing, we could have similar written rules, or even just a few links to good resources like these.
This being said, you mentioned some PRs that are hard to understand. Could you give a few links? It would help me to understand what could be done better
OK so instead of giving a bad example; I thought why not give a good one? i really liked @blurymind’s commits like this one Add support for Piskel layers (+scan/remove unused font/audio) (#731) · 4ian/GDevelop@8a117c1 · GitHub. Before even seeing the change-log for the commit I read the commit message and instantly understood what this commit is about(Ofc the commit title says it all) but the message elaborates it so much more. As a counter-example, I see many commits that are like “Fix Warnings, Fix Typo or Fix … (With no commit message that shows how it was fixed and what events/functions were replaced to fix it)”. I am extremely sorry if i offended anyone. I am not here to nitpick anyone whatsoever.
Alright, I don’t really know how things actually work in the ‘industry’ but I am sure that I can trust your experience I think in conclusion, this topic can be closed as I am convinced with your opinion!
These commits are usually not providing much valuable (they are never shown in the changelog). Fixing a typo is well, fixing a typo, so if you want to learn what was fixed, just look at the code Same for the warning, it’s usually some harmless stuff that was shown in the console, in which case we don’t care about spending time explaining why. If you’re curious, you can always look at the code, which is the source of truth
No worries no one is offended For larger commits, it’s indeed useful to have a better description. If it’s not the case, feel free to signal it - though again the idea is to only do it if it’s worth spending time on this.
One thing we do at work is have some guidelines for commits naming.
[type]: the thing
where type can be the thing the commit does
fix: something not working as expected
style: change buttons size
test: update unit test something something
refactor: component to take x prop
feature: add support for x feature
and so on
Of course you dont want the name to be too long or descriptive either. Its kind of like writing a tweet
Cheers!
For larger commits, it’s indeed useful to have a better description. If it’s not the case, feel free to signal it - though again the idea is to only do it if it’s worth spending time on this.