![]() Easier debugging each commit corresponds to a pull request, so once we find where the bug was introduced, we know which pull request is responsible for it.No time wasted on managing commits inside a pull request discard all the separate commits, so developers don’t have to bother keeping the branch history clean and force pushing.Also, commit messages will be too low-level to be useful. Clean and readable project history developers merge dozens or even hundreds of pull requests every day on a typical project, keeping each commit will make the history enormous and unusable.When I merge pull requests, I always squash all commits into a single commit, and edit the commit message to describe the whole change. ![]() Tip To avoid reviewing the same code on each iteration, mark files as reviewed on GitHub, and they will stay collapsed until the author changes anything inside them. And still, if the changes are minor, I’d group them into a single commit.Īnd if the pull request is too big to review, or contains unrelated changes like refactoring or bugfixes, we should split it into several pull requests. I only use atomic commits after the first code review iteration, so reviewers could better see what was changed since the last review. If some of the changes need additional explanation, code or pull request comments are better places for them - depending on whether they help to understand the code or the reason for the changes.Īlso, atomic commits will likely require rewriting commit history and force-pushing - another thing I don’t want to waste my time on. I’d prefer developers to spend this energy on writing great and helpful pull request descriptions. In the end, that’s what we should care about: how the codebase will look like after we merge the pull request.Ītomic commits read like a novel, taking us behind the scenes of a pull request, and it takes a lot of time to write and maintain them. And most of the time such small changes don’t make sense on their own, without the context of the whole pull request. ![]() Small changes may look okay independently but in the end, they may produce messy code, duplication, or inconsistencies with already existing code - even in the same file. I always review the whole pull request because I don’t want to review tiny changes but the state of the codebase after the change. Some even go further and want each change requested during code review to be in a separate commit. (An illustration from Frederick Vanbrabant’s article.) This approach is called atomic commits, meaning a bigger change is split into multiple smaller independentish changes, and each is represented as a separate commit with a meaningful description of a change. ![]() The latter explains that they want to understand the thought process of a developer, and they prefer to look at smaller changes, and how a developer came up with the solution. ![]() Some folks prefer to check the changes in a pull request as a whole, some prefer to check each commit separately. The first camp was saying that I’m an idiot to use emojis in commit messages, and the second - that it shouldn’t be so easy to break the tool. Funnily, my colleagues split into two camps after this incident. Story time I once broke a deployment tool by adding an emoji into a commit message, and I still consider this a tiny victory in my programming career. I’ve written before on getting code reviewed faster and have published a frontend pull request checklist - check them out! However, many would consider these practices atrocious - read on to find out on which side you are! This approach has worked well for me for many years, and my colleagues seem to be happy with it. I happened to have a somewhat controversial approach to pull requests. ![]()
0 Comments
Leave a Reply. |
AuthorWrite something about yourself. No need to be fancy, just an overview. ArchivesCategories |