5 ms·
I don't do it consciously, but I think I sort of do. First I skim over the full PR; get an impression of the changes. During I mark areas that require more att
by codeptualize 5y ago
I don't do it consciously, but I think I sort of do.
First I skim over the full PR; get an impression of the changes. During I mark areas that require more attention, or any issues I detect. If it's a new feature; I run the code, usually try it out in the app (I do mostly FE and design). Flag any design/usability issues. Basically a bit of QA, try to break it.
After I dive into the code, understand the things I flagged in the first pass. If I can't understand it from the diff, I do some logging, poke at it, and use debuggers. Once I understand everything I go look for bugs, unhandled cases, "risky code". Basically; Will this cause issues now or in the future? Imo this is the most important step.
Lastly I do a quick pass to see if things "make sense"; is there no BS code that needlessly complicates things? Any logic that we already have in our code base? Bad structure? Slow code? Basic sanity checks.
Then I go over my comments (if I have any), make sure they are concise, clear and understandable. I also remove any nitpicks and mark unimportant comments as [minor], or just remove them. Then I write a review comment, usually highlighting the positive things in the PR, as well as summarize my most important findings. Then I submit!