4 ms·
> Then, after they submitted their answer, we would provide them with a thorough and detailed code review (usually by the legendary @mikelehen). > We treated t
by hysan 7y ago
> Then, after they submitted their answer, we would provide them with a thorough and detailed code review (usually by the legendary @mikelehen).
> We treated this code review like a real production code review, and we did it whether or not we planned to move forward with the candidate.
First, thank you for actually investing time in giving feedback to candidates regardless of their performance. Second, did you tell candidates that they would get detailed feedback or a code review as part of the instructions or phone screen? If yes, I figure that this would greatly affect how willing people were to doing the assignment.
I've always hated the thought of take home assignments because I have never been given detailed feedback and only rarely been given any feedback. Requests for feedback are always ignored regardless of performance. I sense that part of why people have greatly soured on take home assignments is because of how lopsided the time investment has become.
- merpnderp 7y agoIf people knew they were going to get a free code review by a notorious developer, they might submit without any intention of taking the job. Having someone who knows there stuff look over your code and give you detailed pointers on what's good and bad, is like finding water in a desert for most devs.
- nefitty 7y agoAny good resources you can recommend to improve my own code review skills? I'm keen on drilling into the most valuable skills needed in the industry right now. It sounds like good code review is super important but lacking.
- ericmcer 7y agoIt's really difficult to actually do a thorough check of logic , exploration of alternate approaches, performance concerns, etc. in all your code reviews, it would be an insane amount of time spent and probably no real return on investment for the company (obviously devs would love to spend a couple hours weighing the merits of different approaches to a problem). I have a list of checks to run through, some general (Sass variables used, reusable code/components used if possible, variables have meaningful names), others more specific to our codebase conventions. I add to the list as new conventions get prioritized and check it. Code review is more about maintaining codebase consistency than suggesting alternate solutions to problems for me.
- nefitty 7y agoAwesome. Thank you for the distillation. I found an insane amount of info as I started searching, so your breakdown of what you consider during reviews helps.
- inertiatic 7y agoA rare post in that it's entirely a coin toss whether it's meant to be irony or not.
- parondea 7y agoGoogle released a Code Review guide which I find extremely useful [1]. [1] https://google.github.io/eng-practices/review/reviewer/ https://google.github.io/eng-practices/review/reviewer/
- bcbrown 7y agoI have two answers to that: Measure and adjust: Find a source of high-quality code reviews; the easiest source may be the high-level engineers at your employer, but if that's not possible/helpful, I'm sure there's free/open sources. Make your own code review before reading others, then compare and contrast. What did they flag that you didn't notice? Can you generalize to categories you could prioritize more? What did you flag that they didn't? When you both flagged the same thing, whose phrasing was more helpful? More personally, here's my approach, which may or may not be helpful for others: * First, skim the entire PR. On a raw, visual level, get a sense of what it looks like. A couple big edits, or a bunch of small updates (like changing a variable/method name, changing an imported library, re-ordering some interaction, etc)? A bunch of indentation changes without much else? An extension of an interface? This is to familiarize myself with the general question of Just What's Going On Here, basic context to use as the foundation for the real review. This should take just a minute or two. * Second, perform a close reading of the diff. Don't worry about high-level conflicts like ordering of operations or other semantic considerations; stick to syntactical-level concerns and context-free mistakes. Are the variables and other symbols well-named? Is exception handling done properly, is this library not initialized correctly? At the "fit and finish"/polishing phase, this might be the majority of the effort. At the early-draft stage, it may be counter-productive to actually note all the possible issues. Either way, to use an analogy to compiling source code, the goal should be to build an AST of the diff, so you can begin to build a model to manipulate through the rest of the review. * Lastly, address the higher-level qualities. Depending on whether you prefer a bottom-up or top-down approach to design, you might order this section differently. I tend to try to identify where on the abstraction scale the highest-leverage feedback resides, and then focus on that level. Depending on context, that could mean discussing which component needs to change, which repository/module this change should live in, which classes should be changed/created, what's the behavior of specific methods, etc. Whatever your feedback, consider what you hope to accomplish with the comment. Are you trying to educate someone about an idiom they don't know about? Ensuring correctness of a complicated interaction? Maintaining a common code style or test coverage threshold? Make maintenance/debugging simpler? Is this something that you view as blocking, or is it more of an FYI? If the latter, it's pretty important to communicate that clearly.
- 7y ago
- dominotw 7y agoIf there is going to be leetcode round after code assigment. I beg ppl to reverse the order, always have leetcode round first and respect candidates time. Way too many companies have leetcode down the line after the candidates have invested considerable time in their process.
- macNchz 7y agoUgh that reminds me of my least favorite interview of all time. Some years ago I did a take home project that I spent several hours on, felt pretty good about the code and how I’d addressed the challenge. A few days later I was called back for a phone interview with the CTO who wanted to do a screenshare leetcode type thing. He had no idea I’d even done the take home task, and wanted me to solve a stupid graph traversal problem requiring the same algorithm I’d used in the take home project.
- inertiatic 7y agoI've had actual comments propagated to me after performing take home assignments and I think not receiving any feedback works better for my own sanity. The comments I personally got back were infuriating enough that convinced me of that.
- james_s_tayler 7y agoI'm curious... what was it about the comments that you found infuriating? And did this provide some signal about the company itself?