I recently wrote a post on how to conduct a constructive code review.
I find that people tend to review the code in a change request and focus solely on the negatives, forgetting that any other kind of a review also highlights the positives.
Personally, I tend to start by looking at the tests as they'll give me more information about what the change is, how it might be implemented and why.
Then, I'll look at the files with the largest number of lines changed as I find these tend to be the files where the main implementation details are located.
As far as a process is concerned, I'll first check whether the piece of work is valid - I've seen changes being made far too often that either duplicate functionality elsewhere, or where the ticket prompting the change is incorrect in some way. Then I check if the code is relatively readable and that it works functionally. If these appear to be fine, I'll then look at edge cases that may not have been considered. I'll also identify any optimizations or ways in which the readability could be improved - however, I tend to recommend that these be put into a follow-up change request rather than preventing the fix/feature from continuing.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
I recently wrote a post on how to conduct a constructive code review.
I find that people tend to review the code in a change request and focus solely on the negatives, forgetting that any other kind of a review also highlights the positives.
Personally, I tend to start by looking at the tests as they'll give me more information about what the change is, how it might be implemented and why.
Then, I'll look at the files with the largest number of lines changed as I find these tend to be the files where the main implementation details are located.
As far as a process is concerned, I'll first check whether the piece of work is valid - I've seen changes being made far too often that either duplicate functionality elsewhere, or where the ticket prompting the change is incorrect in some way. Then I check if the code is relatively readable and that it works functionally. If these appear to be fine, I'll then look at edge cases that may not have been considered. I'll also identify any optimizations or ways in which the readability could be improved - however, I tend to recommend that these be put into a follow-up change request rather than preventing the fix/feature from continuing.