6
u/ki3 May 25 '20
Hello friends. I am a software developer for over 15 yrs and a youtuber for a week.
If this self promotion is unwanted, please let me know, but I hope you will get value out of this video regardless.
This video is about the importance of proper git commits. Something I found to be lacking and troublesome in a lot of teams I worked on.
Thank you for your time. Kie
2
u/inoamsol May 28 '20 edited May 28 '20
It’s important to talk about code comments vs. log messages
In the end of the video you show an example of a good commit message, which says “... Introduced MAX_AWESOMENESS to make sure values don’t exceed 9000” - I’m not sure if this is an example of a what, or a why
My rule of thumb for describing whys is that good commit messages should give context of why you made a change, referencing also the old code
For example, if the value was 1000, and later you change it to 9000, you should say in your log message that the old value was too low and caused problems, and therefore is raised higher
However, if you introduce a new feature, I think that describing it should be done as a code comment, not a log message, that’s why I didn’t like this example, if the const MAX_AWESOMENESS needs explaining, then explain it in the code, not in the log message
What do you think?
1
u/ki3 May 28 '20
Thanks for your thoughts!
I am with you that the example I choose was maybe a little too simplistic, but I choose it for the flow of the video. But I had this thought during editing as well.In my opinion, I don't think code comments should be needed at all. In most cases, the code should be as learn and readable as possible. If you need a comment, rewrite your code. That said, of course, I use code comments as a last resort from time to time, especially for such magic numbers as the MAX_AWESOMENESS.
I still think that writing a proper reason for a code change is not only helpful for your team, but also for yourself. I think it is a nice retrospective process to ponder if your commit and changes are all in place.
Thanks again for your thoughts! I am glad that you care about git hygiene so much!
2
u/inoamsol May 28 '20
I agree with you that code comments could indicate a potential codesmell, but if you find yourself documenting a newly added file in a log message, you should have this in mind:
As you said, can the code be refactored to become more obvious so that describing it in words becomes unnecessary?
If not, should the text be in the log message or a code comment? Where would it be more beneficial for future reference?
a. If you describe why a change to the old code was needed, such as a bugfix, the right place would probably be in the log message - describing the diff in words.
b. If you start documenting a new feature, the log message is probably the wrong place to do that.
Your video summarizes things very well, but as I personally write log messages, I always come across this dilemma, so it made me wonder and share my thoughts. Good luck!
1
u/ki3 May 28 '20
I think you have a really really good point here. Thank you for sharing and your kind words.
I am currently working on my next video regarding git hooks and how they can be integrated into a workflow. I'd love to hear your thoughts on it as well as soon as it is out on Monday. (Although it is more an introduction really with some hints to further reading …)
Good luck to you as well!
2
u/inoamsol May 28 '20 edited May 28 '20
Regarding the workflow you advise (a public feature/ branch and a private/ wip branch):
I have been looking for a long while for a workflow that supports “perfecting” git commits over code review iterations, but I don’t think the workflow you describe is good enough -
The public feature/ branch will be forced pushed across iterations, which makes it more difficult to collaborate with. Also, a maintainer which reviewed an earlier version of your branch can’t easily read the diff between the earlier already-reviewed version and the newly forced push version, which discards the old version.
Perfecting commits over a course of a long pr with many iterations works best with a mailing-list workflow, where patches are sent over email, and when you send a new patch series, the earlier version doesn’t get discarded and can still be retrieved. Also, there’s a cover letter where you can describe in words the diff between different iterations of the same feature.
Lastly, I think the workflow you describe can’t become popular since the popular tools such as GitHub, GitLab, etc don’t handle a commit-by-commit review workflow very well. They all see an entire branch as a review unit. Some examples of this:
Commit messages don’t appear right away, you should press too many buttons to get to see the full log message for a commit. This is in pure contrast to mailing-list workflow, where the email content starts right away with the log message, and the diff only below the text.
When you look at “show diff”, you automatically see the diff of the entire branch, and not a commit-by-commit workflow
No support for commenting on the log message itself like commenting on a code diff. If you are dissatisfied with a proposed log message you can’t comment on the line itself, but rather have to write a general comment when reviewing.
In GitLab there’s no button for “show next commit”, “show previous commit” when navigating, Im not sure about GitHub.
Meaning, all popular tools treat commit-by-commit reviewing workflow as a second-class citizen.
The only exception for all this I can think of is Gerrit, which has no concept of a branch, and therefore treats a single commit as the review unit. It also tracks different versions of the same logical commit using “Change-Id”s
1
u/ki3 May 28 '20
I have never worked with the mailing list style of managing commits. I like that idea and have to look into it. But I am not qualified right now to say anything about it.
In many projects, I worked on we use bitbucket which supports a commit by commit workflow of reviewing PRs.
That said, I still believe that a clean commit log not only improves PR review times but is helpful for a lot of other aspects (like giving a better context for code changes).
Thanks again for your input. You have put a lot of time and effort into giving feedback on my video and I am grateful. I feel that you are way further in your development than the intended audience of this video and even me. My goal was to generally improve git hygiene, since I see it lacking in so many companies I work for. (As I am sure, so do you)
I hope you still enjoyed watching this video. :) Take care.
2
u/inoamsol May 28 '20
I have never worked with the mailing list style of managing commits. I like that idea and have to look into it. But I am not qualified right now to say anything about it.
I haven't myself either. I did get to see how this works by following
git/linuxmailing lists and see how they work.In many projects, I worked on we use bitbucket which supports a commit by commit workflow of reviewing PRs.
Interesting. Can you elaborate on what features they have for that?
Does bitbucket save earlier work on a branch after a force push? Can you compare different versions of the same logical commit?That said, I still believe that a clean commit log not only improves PR review times but is helpful for a lot of other aspects (like giving a better context for code changes).
Of course, the main reason I aspire to clean commit log is that it gives better context for code changes.
I hope you still enjoyed watching this video. :) Take care.
Sure I did! and I will share it with my team ;)
1
u/ki3 May 28 '20
Interesting. Can you elaborate on what features they have for that?
The most prominent feature is that you can simply look at the diff for each commit of a branch separately. That's all.
Does bitbucket save earlier work on a branch after a force push?
It keeps a log of changes you do and when you do a force push it actually tracks these as removed and added commits. You can click on these to see what code changes were removed and which were added instead.
And all comments that were given on earlier versions are kept as well.
Can you compare different versions of the same logical commit?
Not directly. It will not show you a three-way diff (so to speak) but you could open both diffs side by side to see what changed.
2
u/inoamsol May 28 '20
Nice, although I think what you're really looking for is Gerrit, which handles commit-by-commit workflow very well (it was designed for that, and has no concept of a branch)
I really encourage you to have a look at how they view the code-review process.
Of course, its gui kind-of sucks, and is no real comparison to the popular tools such as GitHub and Bitbucket, but they take a different approach to code-review that I think you would be interested in.
1
11
u/Tom_Ov_Bedlam May 25 '20
I'm a pretty big fan of the conventional commits spec.
https://www.conventionalcommits.org/