As I finished the listing functionality of the grid I started to implement the add method of the endpoint. The listing function implementation from the pull request point of view seems pretty good. The commits lead through the reviewer what and why happened. It is easy to review. But, implementing of add feature went sideways. I was distracted by other things during development and couldn’t pay enough attention to commit the code whenever it is logically right. The result is a few commits and a huge one at the end.
It might seems acceptable to have a pull request like this. In my opinion, not really acceptable. Were I the reviewer I would have a short discussion with the developer and try to understand why a commit discipline wasn’t applied.
Let’s take a look at the last commit. It contains implementation at both, client and server side. At server side it contains changes at multiple places like MasterDataHttpClient, Controllers, business logic (kind of repository) and validation. Were I who reviews this I would say a few WTFs, because the way we write code is not about for the computer to understand it. The computer understands the code if it is in a single line without spaces, basically impossible to understand for human being. The way we change the code, the way how changes should be introduced by commits, the variable names and so on must serve the easy understanding of another human being. In many cases, the reviewer. And the developer who is going to make changes in the code a few months or years later.