Never ask for review before ensuring that the new changes can compile and that all unit tests are running. Did you change something major between the client and server communication? Better also check the end-to-end tests then. Don’t waste other people’s time before the code works.
Perhaps some people think that this is an unnecessary point and that it is of cause something any developer is aware of. However, I included this point because I will sometimes experience other developers (and sometimes even myself) being too quick on the trigger and request a review before the code has been build.
In my opinion, this is especially a problem when working on a small issue. You open the issue, change the relatively simple problem, turn to the closest developer and request a code review, thereby interrupting their workflow which we all known can sometimes be a pain in the %#& to get back into.
When then both of you sit in front of the screen, you quickly explain the issue and casually explain how you solved it. The reviewer takes a look at the code and asks if you have added a new unit test to ensure that the bug does not happen again. You then open the testing class, only to realize that you new change breaks several unit tests. – F***ing Great!
Now it has become a question of whether you need to change the unit tests or if the unit tests are ensuring that other functionalities are not broken by “hotshot” developers like you.
Either way, you need to go back and investigate the issue more thoroughly. Unfortunately, you already, stalled the solution to issue, with a premature code review and also interrupted your colleague’s workflow.
Good Job! – Next time, run “Build” first!