From the beginning of the development of Shopsys Framework, one of our main priorities has always been to achieve and maintain an understandable, clean and data-rich git history. The last thing that any developer wants is to end up with code that is incomprehensible or extraneous.
In such cases, debugging and understanding the changes in code can become a nightmare. Because of that we really push ourselves to make every change in the repository clear. We have even created our own guidelines for creating good commits.
Making Commits Perfect
Nobody is flawless and creating perfectly atomic commits with relevant information is very difficult to do on the first try. That’s why we use a few git abilities to help us. We are constantly rebasing our branches onto a current master, rewriting our commit messages, moving changes into different commits, and squashing changes into former commits when it makes sense. These actions are usually done several times during the development and code review process.
GitHub Code Reviews
While these techniques worked for us really well at first, after publicly releasing Shopsys Framework on GitHub there were a few problems which were brought to our attention.
Firstly, there are pull requests from the community, where we request changes on commit messages and content inside of commits. This can sometimes hurt motivation, and discourage developers from contributing. On the other hand, we want contributions from our community to be understandable and consistent with other commits, too.
Then there is a quality of our code reviews — while GitHub is a great tool for code reviews, it has some difficulty handling changes that are not pullable. So, after rebasing branch onto a new master and changing the order or content of commits, all of the comments and recommendations that have been submitted by reviewers become hidden or get lost entirely. This means that after changing the history during the code review, the reviewer cannot get information about the requested changes any longer and must go through the whole review process again.
!Fixup Solution
While we want contributors to stick primarily to developing for us without spending too much time dealing with commits and history, we generally do not request these types of changes from them. We only request changes of the code, and after approving it for merging into the master, our developers take care of the history themselves.
For the quality of code reviews within our team, we came up with a solution using another great git ability. All changes requested in review are added into the branch as fixup commits.
Fixup commits are pointing to other commits that they belong with, so while all the changes after review are done in new commits, they are prepared for rebasing and squashing all relevant changes together. With this approach, we are not forced to rebase and push force our changes into already existing commits during the code review and a reviewer is able to check the new changes easily.
Conclusion
Clean and understandable git history including atomic commits is really important and worth maintaining. On the other hand, high-quality requirements for commits could be a barrier for contributors to be creative. Do not force your community to maintain a git history by themselves internally. For internal pull requests, use fixup commits that make code reviews on GitHub comfortable and squash them all together after changes are approved for merging into the master. You can see how we maintain our history for yourself on our GitHub.
