8 ms·

How to Review Code as a Junior Developer

133 points by allenleein 5y ago

wizwit999 5y ago
I see if here a lot and I don't think it's the right mindset to think of some dichotomy of junior and senior developers (unless you truly are in some sort of apprenticeship program). Some of the poorest performing engineers I met at faang were high in level. You should act and carry yourself the same whether you're senior or junior in level.
- siva7 5y ago
  it's usually there to justify the wage band in companies who also hire fresh grads as you can't pay someone fresh out of school the same as someone with 10 years of experience. In companies who only hire seniors you ususally don't see this distinction anymore.
  - AmericanChopper 5y ago
    Do people really believe there’s no relationship between experience and competency?
    - MrPatan 5y ago
      There is indeed, but careful with words. "Experience" meaning "practice" is the only thing that will make you competent. "Experience" meaning "time passed while on payroll at an org" is not. One is easier to measure than the other.
      - AmericanChopper 5y ago
        Sure, you could do a job for 40 years, and be useless at it every single day of your life. Which is why you want to try and do your best at screen candidates, and promote based on competency. But distinguishing ICs based on some model of seniority isn’t “usually there to justify the wage band in companies who also hire fresh grads as you can't pay someone fresh out of school the same as someone with 10 years of experience”, it’s done because the person with more competency is more valuable to the organisation.
      - konschubert 5y ago
        There are also differences in aptitude. The same amount of practice may produce very different outcomes in different developers.
        junon 5y ago
        But "everyone can be a coder", so said the code camps!
  - thn-gap 5y ago
    I disagree here. I've seen many cases in FAANG of someone being hired at a lower level than they should, just because the company gets away with it, and the hired person still gets better compensation than they were getting before. So you can certainly have both newgrads and people with years of experience being paid the same. Although not for the reasons that you initially mentioned.
- Cthulhu_ 5y ago
  Yeah, skill differences should be obvious but also considered a problem, since in the end we're all in this together. I don't know how it is in the US, but over here it's really hard to find good developers, let alone ones that are willing and able to stick with one company for a longer period of time, so you really need to get people on a 'lower' level up to your level. I don't get it either. I'd like to get some recognition for 10+ years of experience, but for now I'm just "a developer". At the same time I see job openings for senior developers asking for only three years of experience. ???
  - TeMPOraL 5y ago
    > I'd like to get some recognition for 10+ years of experience, but for now I'm just "a developer". At the same time I see job openings for senior developers asking for only three years of experience. I think the trick is to find a company to hire you and convince them to bump you to "senior developer", even on paper. All that matters is the title, because once you get it, you can put it on your résumé and apply directly for that role, and other companies won't be asking much questions. Repeat for principal developer, but keep in mind the risk of not being able to find an actual coding job. > I don't know how it is in the US, but over here it's really hard to find good developers, let alone ones that are willing and able to stick with one company for a longer period of time, so you really need to get people on a 'lower' level up to your level. US is still vacuuming up software talent from all other the world. Anecdote: of my real-life software developer friends and acquaintances, a good half of those I consider to be good developers are either working for FAANG or running a startup and either receiving or applying for YC funding. As for sticking with one company, this won't be solved while typical raises are non-existent. If the choice is ~2% per year of raise vs. 50-400% salary increase by jumping ship, good developers will keep jumping ship until they're either comfortable or hit some salary ceiling. Another point I tentatively bring up is, career growth opportunities tend not to be aligned with retaining good developers. Take a look at Dropbox's Career Framework, currently on the frontpage[0], or any other typical SWE leveling system. Senior+ is frequently no longer software development, but management in disguise. In other words, for software developers who want to get better at the technical aspects, career ends at the senior level. At this point, it's only sane to jump to a company that will at least pay more. This is something I frankly don't understand: a senior software developer is roughly the level where you can call a programmer competent. It's also a level where companies want them to stop programming and start managing juniors. This feels like a huge waste in terms of productivity, letting someone gain proficiency in a skill, and then prevent them from using it. But I guess the market likes it that way, so ¯\_(ツ)_/¯. -- [0] - https://news.ycombinator.com/item?id=27817519 https://news.ycombinator.com/item?id=27817519
- dkdbejwi383 5y ago
  The two senior devs I report to routinely: - Approve one another's PRs (which are either 1 or 2 line things, or 2000 line behemoths. Never in between) without a review, or approve ignoring comments left by others - Just comment out tests that won't pass because of a change they made, rather than fix it - Because of the first point, sloppy mistakes and debug stuff (print statements) gets left in the codebase - Follow poor code health practices, like disabling deprecation warnings instead of updating deprecated code Whereas most of the mid & junior level devs in the team spend time writing tests, suggest new test cases for one another in PRs, discuss clarity of syntax, try to keep their changes minimal and understandable. I'm only really sticking around at this point until the pandemic situation has calmed down a bit more, and because I don't want to have too many short experiences on my CV.
  - macando 5y ago
    Just like in this video: https://youtu.be/rR4n-0KYeKQ https://youtu.be/rR4n-0KYeKQ They're probably good developers but they lost motivation somewhere along the way.
  - nwmcsween 5y ago
    Those people are senior in title only, the senior title doesn't mean good or competent
- m12k 5y ago
  I think it's a sensible distinction (or two data points to help you map out a spectrum that people can evolve along), but it needs to be based on actual professionalism and skill, not just seniority. Like you experienced, some people don't grow, and their "10 years of experience" is just 1 year's experience repeated 10 times. A senior dev is experienced, pragmatic, effective (at both tech and communication), self-directing and low-maintenance (from a management perspective), and always leaves the campsite cleaner than they found it. They're someone a manager knows they can point at a general problem area, and feel confident that "good things will happen". By comparison, a junior dev needs much more hand-holding, and "pre-chewing of their food". They are less experienced, so have less of a sense of what the core of a problem is, what goes into identifying it, what a good solution looks like, how to bring it about, what techniques can go into that, and which people need to be involved at what point. Their tasks need to be broken down and specified more, they might need help along the way that they don't even know they should ask for, and their solutions need review. Being able to identify and classify these traits is helpful - both to help junior devs succeed despite their larger need for assistance, but to give all devs something to aspire to - everyone can always get better.
- munchbunny 5y ago
  I think it’s a useful shorthand to represent a set of traits and capabilities that good developers acquire over time. Just because there are junior developers punching above their weight or senior developers who don’t deserve the job title doesn’t mean the label isn’t useful. I do agree though that regardless of your level or the level of the person you’re talking to, you should be respectful, humble, and generous. But the junior/senior labels definitely exist on a spectrum. I have definitely moved from a more junior place on it towards a more senior place on it over time, even if exactly where I stand on it is for my colleagues to judge.
karmakaze 5y ago
Good advice: > “Just ask questions.” Some of the best junior devs I work with do this really well, and the more time I spend explaining in detail the more fine details are discovered. More senior levels should fill in the blanks, from proposing alternatives, to making recommendations or enforcing certain aspects.
erdo 5y ago
How to x articles usually annoy me, but I think this was great advice. And not just for junior developers
- another-dave 5y ago
  Agreed. I wish a lot more so-called senior developers would start their critique with "Why did you do X?" rather than "You should've done Y". Sure, if there's an obvious error it's good to just go ahead & point it out, but you're often landed with the negative criticism first, simply because your colleague didn't understand the problem space enough. Your code might still need work since they didn't — e.g. better naming/test cases/comments — but it sets a completely different tone.
vncecartersknee 5y ago
Even as someone thats only just above a junior engineer, I feel like I've been made to feel stupid or that I was wasting other peoples time or such for "just asking questions" so many times. I'm extremely reluctant to do so tbh. There have of course been people who encouraged and welcomed questions (for whom I'm extremely grateful) but it's the negative experiences that stand out. It's really been a major contributor to my loss of interest and passion for this industry over the last few years I think.
- robaato 5y ago
  Well there are questions and questions, and also many ways of and times of asking them. For example, and oldie but goodie is: http://www.catb.org/~esr/faqs/smart-questions.html http://www.catb.org/~esr/faqs/smart-questions.html That might be overkill for internal review situations, but it is worth reading and bearing in mind. Context is all. Sometimes it is the problem of the person you asked. Sometimes it is just the wrong time. Sometimes they are under pressure or just feeling a bit grumpy and snap at you. Sometimes there really is a "stoopid question"! Sometimes you need to try asking the rubber duck first (https://en.wikipedia.org/wiki/Rubber_duck_debugging https://en.wikipedia.org/wiki/Rubber_duck_debugging)! Keep your chin up and don't take it personally!
- mypastself 5y ago
  Not to discount your experience, and it’s likely this doesn’t apply to you at all, but I don’t think welcoming all types of questions should be expected in this type of environment. There is usually a baseline level of knowledge and skill expected even from a fresh coding bootcamp grad. If they’re not familiar with certain fundamental concepts, answering some of their questions can be frustrating for more senior colleagues. Business domain knowledge or company-specific tooling is a different thing, but even there, some people tend to avoid note-taking (or looking up documentation), which can result in them repeatedly asking the exact same questions. Of course, even in those cases, some degree of tact and patience on part of the seniors should be expected, but as harsh as that sounds, there is such a thing as a stupid question.
  - skinkestek 5y ago
thesnide 5y ago
I think it all boils down to ask the "why" instead of the "what". And the answer to those questions is a good signal to classify "seniors" from "experts". Seniors explain, experts know.
deleted 5y ago
[deleted]
Cthulhu_ 5y ago
I think another Important lesson - besides the "be curious" advice given by this article - is that you should never trust the author. The last time I was involved in code reviews, years ago, I felt like most of my PRs were approved after a quick scan simply because I wrote the code and they trusted me. Bad idea. If I ever go to an environment like that again, I wonder if there's a way to make code reviews (both reviewers and reviewees) anonymous, to prevent bias like that.
ionwake 5y ago
What I read is positive it’s awesome. On a related note I can’t wrap my head around how I had to grift over a decade to get roles which would pay as much ( and include this level of responsibility and respect) in the UK. Even now I would be aghast with surprise if a company asked me to spend time writing a blog during company time, pleasantly surprised if I was asked to code review. And I mean surprised. Even with my level of experience. ( normally there would be a silo coder who has been there a decade who is the only one allowed to review code). I’m not saying it’s unfair I’m just pointing out how different a programmers life can be if they don’t slot it into a FAANG company and ends up working at small ones in Europe. Any thoughts are welcome. Loved the article and the vibe I think it’s great to include junior devs, just pointing out the disparity in wealth and working conditions within 1st world countries tech world
- doteka 5y ago
  I’m truly sorry you had this experience. I must say it doesn’t match mine at all: code review was a major thing since my first day working as a dev, up to this day as an eng manager. My engineers are highly encouraged to write blog posts, it’s a great way to market the company and help with hiring. All of these companies I worked for have been fairly ordinary startups/corporates in the Netherlands too.
  - ionwake 5y ago
    There is no time to write blog posts , every engineer is there to try and complete as many tickets as possible before the end of the sprint. I’m glad it’s different in your place. I’ve worked at tons of different companies and it was always the same. Careful what companies you move to. As a final note I was not expecting any empathy it’s taken me aback slightly. I’m ok now guys I sorted my life out with another revenue stream thanks for replies.
- golergka 5y ago
  > On a related note I can’t wrap my head around how I had to grift over a decade to get roles which would pay as much ( and include this level of responsibility and respect) in the UK. I'm not even in western europe, and I had the same feeling, but looks like with advent of remote it's rapidly changing. Not in FAANGs yet, but I think they'll come around to it, too.
lmilcin 5y ago
From my pov as a tech lead, most of these tips apply to any dev reviewing the code, not just junior. I would even say that juniors tend to do better at these tips than seniors, especially asking questions. I don't see junior devs having too much of a problem asking questions. They are expected to and reminded if they don't. I see senior devs having problem asking questions. And then nobody points it out to them. Calibrating the feedback? Seniors tend to run on an autopilot and routine and some just don't notice they need to keep doing these things.
dusted 5y ago
We will not submit a change that's not been reviewed, meaning that everyone does multiple reviewss per day. While it may not catch all the bugs (it will catch most of the typos though) it does help the style and readability a lot, it makes everyone more familiar with the codebase and it often presents learning opportunities for everyone. It was a bit intimidating at first, being the new guy, and at first I would read through things and mostly say "+1, I can't find any obvious fault with this", and gradually, I became confident enough to start asking questions, and started to catch logic errors and uncover edge-cases. But boy do I ask a lot of questions that have some trivial answer that I just didn't know.. Learning not to care if others think you're ignorant, or maybe, accepting that to some degree, everyone are.. Was key for me to get more out of reviewing.
ChrisMarshallNY 5y ago
I’ve always found that asking questions about my work is a good way to find design issues. Doing things like looking at code coverage for tests, is a good way to look at the implementation, but it doesn’t inspect the design. The article describes a great way to teach newer folks, while forcing older folks to review their architectural decisions. A good way to improve, once we have achieved a certain level, is to teach, and that works by forcing us to review what we do, from a new perspective.
anothernewdude 5y ago
Pinterest have engineers? I thought they were primarily a black SEO shop that poisoned google's image search results.
javier10e6 5y ago
One question from a Junior Dev or any dev for that matter with multiple possible outcomes depending: Q: Hi <insert dev name here>, say, why are you asking in my review if the class I introduced is really necessary? Answer from the Paranoia Distroya. PD: (Without taking their eyes from the screen or their hands off the keyboard). What is exactly the reason for the class? You: Refactor common functionality. PD: Well, long pause while typing really fast, that common functionality may change. You: May? PD: (long pause) I'll take a another look.... You leave after an awkward silence, next day your check-in is reverted and a new check in from PD has the module updated and the comment in the code review marked as resolved. Answer from Mr. Rogers. MR: See, base classes are like like a regular class but they are also special. They provide an added layer of encapsulation. You: So.... MR: I tell you what, lets go to your desk and I'll help you work on the base class and update the code with something that is much better, then we will update your review and having it all tied up before lunch. You and Mr. Rogers sit with you and tells what to change while he proselytize on the amazing powers of some editor. You try not to sight. He approves the review with a smiley emoji. Answer from My Friend From Across the World (over SMS). MFFAW: What review? Can you give me review no? You: DFDDS-4322343 MFFAW: Code change, check co: 5234543 You: I see commit, what of it? MFFAW: Library removed, NA. You: What library? MFFAW: brb You don't hear back from MFFAW, during scrum you ask again what to do about the code review, MFFAW gets the story assigned and the review is closed. The code is checked in. A week later MFFAW updates the code and the base class you added is gone. Take away: Ask questions; be ready to roll with the punches.
z5h 5y ago
20+ years as some level of engineer. I’m a fan of code review culture. Tip: No matter junior or senior you are, NEVER be afraid or embarrassed to say you are having trouble understanding something and then figure out where that trouble is coming from.
- FartyMcFarter 5y ago
  > NEVER be afraid or embarrassed to say you are having trouble understanding something Especially since other people reading the code will probably run into the same issues later on. A code review is the best time for such concerns to come up - the code is still fresh in the author's mind, and it can easily be made clearer or commented better.
andai 5y ago
I work alone and expected to learn how to review my own code from this, but there was actually nothing about reviewing code in the article and all the advice only works if you're on a team. Something I've found surprisingly useful is to turn my code into a tutorial. I've been working on an educational product recently and I've found writing explanations of my code extremely useful for improving the code; I've made dozens and dozens of changes even after I thought the code was "done" and it was time to begin writing the text. I'm recording videos based on the text and that's making me realize things about the code that I didn't realize just by writing about it. It's as though, for me, writing and speaking activate different ways of thinking.
- mynameismon 5y ago
  Related (and probably helpful): https://rubberduckdebugging.com/ https://rubberduckdebugging.com/
kirkules 5y ago
I think this is great advice. My perspective on code reviews derives from the fact that really, a pull request/changelist is a conversation of complete ownership of code by the author to joint ownership by author and team/organization. If you're any developer, junior or otherwise, participating in a code review simply means helping that ownership conversion happen however it needs to, whether that means finding and fixing mistakes in the code itself, bringing your own understanding of the code up to speed, or helping to clarify and unify the strategies and logic represented in the code with what will work for the team, which includes you.
thanksforfish 5y ago
I saw a thread recently that suggested reviewers should be running the code, not just reading it. Does anyone here run code as well? If so, I'd be interested in hearing about the experience. I've almost always just read the code and "ran it in my head".
- mattm 5y ago
  I will do this sometimes if I don't understand how something works and want to confirm it on my own before asking about it. I'll also run the code if I suspect something was missed and want to confirm before bringing it up.
- silveraxe93 5y ago
  You need to be careful with that. The brain is amazing at reading what you _expect_, not what's actually written. It's the same as proofreading. Your brain just papers over repeated words or typos, or off-by-one errors. Obviously there's a middle ground and it's unreasonable to ask someone to run every single line of code, but it's good to be aware of that.
werber 5y ago
The ask questions section is so important, and not just for Junior devs. I used to be afraid of asking when I wasn’t sure I understood and I’ve learned so much about how to do my job and how people think from always asking now. I wish I hadn’t been so arrogant and lacking confidence earlier in my career
thdc 5y ago
> why they added that piece of logic in that file instead of further downstream This is the kind of thing that has always bothered me about code reviews. I'm confident that my teammates can write working code or any (obvious) bugs can be caught through code review, but is it possible there is a better solution by modifying code somewhere else? For example, there's a bug that is causing several issues, and there is a report for one of those issues. A patch is submitted to fix the issue without fixing the root cause and you're reviewing it. You verify the code looks good and that the issue is fixed - not knowing that the root cause itself is not fixed due to unfamiliarity with the system and it not being anywhere in the patch - and approve it. How much time should be spent looking at whats not in the patch for alternate solutions? How would I know if something could be updated "downstream"? Personally, I used to do it but not so much anymore.
MeinBlutIstBlau 5y ago
My issue with learning codebases is when there is no documentation on what can be passed through as variables in an API. I mean it's fantastic my coworker wrote something to simplify REST retrievals for us, but what format is "string oldDate" in? Is it YYYY-mm-DD? mm/DD? I really hate learning APIs if there is no example of how it can be used. Microsoft has done a stellar job on their documentation but it still isn't enough at times. The "just do it" or "it just works" mentality with code is the #1 thing I hate when using other people's code. Half the time I have to read stuff directly out of their own codebases and actually run it to even figure it out. All because they were too lazy to write a comment somewhere to say "an example of this variable is: ...". In theory, I should be able to use your code right away, not pick at it for hours and go "ohhhh now I see."
rcfaj7obqrkayhn 5y ago
Good advice, but maybe it's hard for people to follow, especially in companies/orgs where people focus on their own promotions so hoarding knowledge = job security

133 points by allenleein 5y ago

I see if here a lot and I don't think it's the right mindset to think of some dichotomy of junior and senior developers (unless you truly are in some sort of apprenticeship program). Some of the poorest performing engineers I met at faang were high in level. You should act and carry yourself the same whether you're senior or junior in level.

siva7 5y ago
it's usually there to justify the wage band in companies who also hire fresh grads as you can't pay someone fresh out of school the same as someone with 10 years of experience. In companies who only hire seniors you ususally don't see this distinction anymore.
- AmericanChopper 5y ago
  Do people really believe there’s no relationship between experience and competency?
  - MrPatan 5y ago
    There is indeed, but careful with words. "Experience" meaning "practice" is the only thing that will make you competent. "Experience" meaning "time passed while on payroll at an org" is not. One is easier to measure than the other.
    - AmericanChopper 5y ago
      Sure, you could do a job for 40 years, and be useless at it every single day of your life. Which is why you want to try and do your best at screen candidates, and promote based on competency. But distinguishing ICs based on some model of seniority isn’t “usually there to justify the wage band in companies who also hire fresh grads as you can't pay someone fresh out of school the same as someone with 10 years of experience”, it’s done because the person with more competency is more valuable to the organisation.
    - konschubert 5y ago
      There are also differences in aptitude. The same amount of practice may produce very different outcomes in different developers.
      - junon 5y ago
        But "everyone can be a coder", so said the code camps!
- thn-gap 5y ago
  I disagree here. I've seen many cases in FAANG of someone being hired at a lower level than they should, just because the company gets away with it, and the hired person still gets better compensation than they were getting before. So you can certainly have both newgrads and people with years of experience being paid the same. Although not for the reasons that you initially mentioned.
Cthulhu_ 5y ago
Yeah, skill differences should be obvious but also considered a problem, since in the end we're all in this together. I don't know how it is in the US, but over here it's really hard to find good developers, let alone ones that are willing and able to stick with one company for a longer period of time, so you really need to get people on a 'lower' level up to your level. I don't get it either. I'd like to get some recognition for 10+ years of experience, but for now I'm just "a developer". At the same time I see job openings for senior developers asking for only three years of experience. ???
- TeMPOraL 5y ago
  > I'd like to get some recognition for 10+ years of experience, but for now I'm just "a developer". At the same time I see job openings for senior developers asking for only three years of experience. I think the trick is to find a company to hire you and convince them to bump you to "senior developer", even on paper. All that matters is the title, because once you get it, you can put it on your résumé and apply directly for that role, and other companies won't be asking much questions. Repeat for principal developer, but keep in mind the risk of not being able to find an actual coding job. > I don't know how it is in the US, but over here it's really hard to find good developers, let alone ones that are willing and able to stick with one company for a longer period of time, so you really need to get people on a 'lower' level up to your level. US is still vacuuming up software talent from all other the world. Anecdote: of my real-life software developer friends and acquaintances, a good half of those I consider to be good developers are either working for FAANG or running a startup and either receiving or applying for YC funding. As for sticking with one company, this won't be solved while typical raises are non-existent. If the choice is ~2% per year of raise vs. 50-400% salary increase by jumping ship, good developers will keep jumping ship until they're either comfortable or hit some salary ceiling. Another point I tentatively bring up is, career growth opportunities tend not to be aligned with retaining good developers. Take a look at Dropbox's Career Framework, currently on the frontpage[0], or any other typical SWE leveling system. Senior+ is frequently no longer software development, but management in disguise. In other words, for software developers who want to get better at the technical aspects, career ends at the senior level. At this point, it's only sane to jump to a company that will at least pay more. This is something I frankly don't understand: a senior software developer is roughly the level where you can call a programmer competent. It's also a level where companies want them to stop programming and start managing juniors. This feels like a huge waste in terms of productivity, letting someone gain proficiency in a skill, and then prevent them from using it. But I guess the market likes it that way, so ¯\_(ツ)_/¯. -- [0] - https://news.ycombinator.com/item?id=27817519 https://news.ycombinator.com/item?id=27817519
dkdbejwi383 5y ago
The two senior devs I report to routinely: - Approve one another's PRs (which are either 1 or 2 line things, or 2000 line behemoths. Never in between) without a review, or approve ignoring comments left by others - Just comment out tests that won't pass because of a change they made, rather than fix it - Because of the first point, sloppy mistakes and debug stuff (print statements) gets left in the codebase - Follow poor code health practices, like disabling deprecation warnings instead of updating deprecated code Whereas most of the mid & junior level devs in the team spend time writing tests, suggest new test cases for one another in PRs, discuss clarity of syntax, try to keep their changes minimal and understandable. I'm only really sticking around at this point until the pandemic situation has calmed down a bit more, and because I don't want to have too many short experiences on my CV.
- macando 5y ago
  Just like in this video: https://youtu.be/rR4n-0KYeKQ https://youtu.be/rR4n-0KYeKQ They're probably good developers but they lost motivation somewhere along the way.
- nwmcsween 5y ago
  Those people are senior in title only, the senior title doesn't mean good or competent
m12k 5y ago
I think it's a sensible distinction (or two data points to help you map out a spectrum that people can evolve along), but it needs to be based on actual professionalism and skill, not just seniority. Like you experienced, some people don't grow, and their "10 years of experience" is just 1 year's experience repeated 10 times. A senior dev is experienced, pragmatic, effective (at both tech and communication), self-directing and low-maintenance (from a management perspective), and always leaves the campsite cleaner than they found it. They're someone a manager knows they can point at a general problem area, and feel confident that "good things will happen". By comparison, a junior dev needs much more hand-holding, and "pre-chewing of their food". They are less experienced, so have less of a sense of what the core of a problem is, what goes into identifying it, what a good solution looks like, how to bring it about, what techniques can go into that, and which people need to be involved at what point. Their tasks need to be broken down and specified more, they might need help along the way that they don't even know they should ask for, and their solutions need review. Being able to identify and classify these traits is helpful - both to help junior devs succeed despite their larger need for assistance, but to give all devs something to aspire to - everyone can always get better.
munchbunny 5y ago
I think it’s a useful shorthand to represent a set of traits and capabilities that good developers acquire over time. Just because there are junior developers punching above their weight or senior developers who don’t deserve the job title doesn’t mean the label isn’t useful. I do agree though that regardless of your level or the level of the person you’re talking to, you should be respectful, humble, and generous. But the junior/senior labels definitely exist on a spectrum. I have definitely moved from a more junior place on it towards a more senior place on it over time, even if exactly where I stand on it is for my colleagues to judge.

karmakaze 5y ago

Good advice: > “Just ask questions.” Some of the best junior devs I work with do this really well, and the more time I spend explaining in detail the more fine details are discovered. More senior levels should fill in the blanks, from proposing alternatives, to making recommendations or enforcing certain aspects.

erdo 5y ago

How to x articles usually annoy me, but I think this was great advice. And not just for junior developers

another-dave 5y ago
Agreed. I wish a lot more so-called senior developers would start their critique with "Why did you do X?" rather than "You should've done Y". Sure, if there's an obvious error it's good to just go ahead & point it out, but you're often landed with the negative criticism first, simply because your colleague didn't understand the problem space enough. Your code might still need work since they didn't — e.g. better naming/test cases/comments — but it sets a completely different tone.

vncecartersknee 5y ago

Even as someone thats only just above a junior engineer, I feel like I've been made to feel stupid or that I was wasting other peoples time or such for "just asking questions" so many times. I'm extremely reluctant to do so tbh. There have of course been people who encouraged and welcomed questions (for whom I'm extremely grateful) but it's the negative experiences that stand out. It's really been a major contributor to my loss of interest and passion for this industry over the last few years I think.

robaato 5y ago
Well there are questions and questions, and also many ways of and times of asking them. For example, and oldie but goodie is: http://www.catb.org/~esr/faqs/smart-questions.html http://www.catb.org/~esr/faqs/smart-questions.html That might be overkill for internal review situations, but it is worth reading and bearing in mind. Context is all. Sometimes it is the problem of the person you asked. Sometimes it is just the wrong time. Sometimes they are under pressure or just feeling a bit grumpy and snap at you. Sometimes there really is a "stoopid question"! Sometimes you need to try asking the rubber duck first (https://en.wikipedia.org/wiki/Rubber_duck_debugging https://en.wikipedia.org/wiki/Rubber_duck_debugging)! Keep your chin up and don't take it personally!
mypastself 5y ago
Not to discount your experience, and it’s likely this doesn’t apply to you at all, but I don’t think welcoming all types of questions should be expected in this type of environment. There is usually a baseline level of knowledge and skill expected even from a fresh coding bootcamp grad. If they’re not familiar with certain fundamental concepts, answering some of their questions can be frustrating for more senior colleagues. Business domain knowledge or company-specific tooling is a different thing, but even there, some people tend to avoid note-taking (or looking up documentation), which can result in them repeatedly asking the exact same questions. Of course, even in those cases, some degree of tact and patience on part of the seniors should be expected, but as harsh as that sounds, there is such a thing as a stupid question.
- skinkestek 5y ago

thesnide 5y ago

I think it all boils down to ask the "why" instead of the "what". And the answer to those questions is a good signal to classify "seniors" from "experts". Seniors explain, experts know.

deleted 5y ago

[deleted]

Cthulhu_ 5y ago

I think another Important lesson - besides the "be curious" advice given by this article - is that you should never trust the author. The last time I was involved in code reviews, years ago, I felt like most of my PRs were approved after a quick scan simply because I wrote the code and they trusted me. Bad idea. If I ever go to an environment like that again, I wonder if there's a way to make code reviews (both reviewers and reviewees) anonymous, to prevent bias like that.

ionwake 5y ago

What I read is positive it’s awesome. On a related note I can’t wrap my head around how I had to grift over a decade to get roles which would pay as much ( and include this level of responsibility and respect) in the UK. Even now I would be aghast with surprise if a company asked me to spend time writing a blog during company time, pleasantly surprised if I was asked to code review. And I mean surprised. Even with my level of experience. ( normally there would be a silo coder who has been there a decade who is the only one allowed to review code). I’m not saying it’s unfair I’m just pointing out how different a programmers life can be if they don’t slot it into a FAANG company and ends up working at small ones in Europe. Any thoughts are welcome. Loved the article and the vibe I think it’s great to include junior devs, just pointing out the disparity in wealth and working conditions within 1st world countries tech world

doteka 5y ago
I’m truly sorry you had this experience. I must say it doesn’t match mine at all: code review was a major thing since my first day working as a dev, up to this day as an eng manager. My engineers are highly encouraged to write blog posts, it’s a great way to market the company and help with hiring. All of these companies I worked for have been fairly ordinary startups/corporates in the Netherlands too.
- ionwake 5y ago
  There is no time to write blog posts , every engineer is there to try and complete as many tickets as possible before the end of the sprint. I’m glad it’s different in your place. I’ve worked at tons of different companies and it was always the same. Careful what companies you move to. As a final note I was not expecting any empathy it’s taken me aback slightly. I’m ok now guys I sorted my life out with another revenue stream thanks for replies.
golergka 5y ago
> On a related note I can’t wrap my head around how I had to grift over a decade to get roles which would pay as much ( and include this level of responsibility and respect) in the UK. I'm not even in western europe, and I had the same feeling, but looks like with advent of remote it's rapidly changing. Not in FAANGs yet, but I think they'll come around to it, too.

lmilcin 5y ago

From my pov as a tech lead, most of these tips apply to any dev reviewing the code, not just junior. I would even say that juniors tend to do better at these tips than seniors, especially asking questions. I don't see junior devs having too much of a problem asking questions. They are expected to and reminded if they don't. I see senior devs having problem asking questions. And then nobody points it out to them. Calibrating the feedback? Seniors tend to run on an autopilot and routine and some just don't notice they need to keep doing these things.

dusted 5y ago

We will not submit a change that's not been reviewed, meaning that everyone does multiple reviewss per day. While it may not catch all the bugs (it will catch most of the typos though) it does help the style and readability a lot, it makes everyone more familiar with the codebase and it often presents learning opportunities for everyone. It was a bit intimidating at first, being the new guy, and at first I would read through things and mostly say "+1, I can't find any obvious fault with this", and gradually, I became confident enough to start asking questions, and started to catch logic errors and uncover edge-cases. But boy do I ask a lot of questions that have some trivial answer that I just didn't know.. Learning not to care if others think you're ignorant, or maybe, accepting that to some degree, everyone are.. Was key for me to get more out of reviewing.

ChrisMarshallNY 5y ago

I’ve always found that asking questions about my work is a good way to find design issues. Doing things like looking at code coverage for tests, is a good way to look at the implementation, but it doesn’t inspect the design. The article describes a great way to teach newer folks, while forcing older folks to review their architectural decisions. A good way to improve, once we have achieved a certain level, is to teach, and that works by forcing us to review what we do, from a new perspective.

anothernewdude 5y ago

Pinterest have engineers? I thought they were primarily a black SEO shop that poisoned google's image search results.

javier10e6 5y ago

One question from a Junior Dev or any dev for that matter with multiple possible outcomes depending: Q: Hi <insert dev name here>, say, why are you asking in my review if the class I introduced is really necessary? Answer from the Paranoia Distroya. PD: (Without taking their eyes from the screen or their hands off the keyboard). What is exactly the reason for the class? You: Refactor common functionality. PD: Well, long pause while typing really fast, that common functionality may change. You: May? PD: (long pause) I'll take a another look.... You leave after an awkward silence, next day your check-in is reverted and a new check in from PD has the module updated and the comment in the code review marked as resolved. Answer from Mr. Rogers. MR: See, base classes are like like a regular class but they are also special. They provide an added layer of encapsulation. You: So.... MR: I tell you what, lets go to your desk and I'll help you work on the base class and update the code with something that is much better, then we will update your review and having it all tied up before lunch. You and Mr. Rogers sit with you and tells what to change while he proselytize on the amazing powers of some editor. You try not to sight. He approves the review with a smiley emoji. Answer from My Friend From Across the World (over SMS). MFFAW: What review? Can you give me review no? You: DFDDS-4322343 MFFAW: Code change, check co: 5234543 You: I see commit, what of it? MFFAW: Library removed, NA. You: What library? MFFAW: brb You don't hear back from MFFAW, during scrum you ask again what to do about the code review, MFFAW gets the story assigned and the review is closed. The code is checked in. A week later MFFAW updates the code and the base class you added is gone. Take away: Ask questions; be ready to roll with the punches.

z5h 5y ago

20+ years as some level of engineer. I’m a fan of code review culture. Tip: No matter junior or senior you are, NEVER be afraid or embarrassed to say you are having trouble understanding something and then figure out where that trouble is coming from.

FartyMcFarter 5y ago
> NEVER be afraid or embarrassed to say you are having trouble understanding something Especially since other people reading the code will probably run into the same issues later on. A code review is the best time for such concerns to come up - the code is still fresh in the author's mind, and it can easily be made clearer or commented better.

andai 5y ago

I work alone and expected to learn how to review my own code from this, but there was actually nothing about reviewing code in the article and all the advice only works if you're on a team. Something I've found surprisingly useful is to turn my code into a tutorial. I've been working on an educational product recently and I've found writing explanations of my code extremely useful for improving the code; I've made dozens and dozens of changes even after I thought the code was "done" and it was time to begin writing the text. I'm recording videos based on the text and that's making me realize things about the code that I didn't realize just by writing about it. It's as though, for me, writing and speaking activate different ways of thinking.

mynameismon 5y ago
Related (and probably helpful): https://rubberduckdebugging.com/ https://rubberduckdebugging.com/

kirkules 5y ago

I think this is great advice. My perspective on code reviews derives from the fact that really, a pull request/changelist is a conversation of complete ownership of code by the author to joint ownership by author and team/organization. If you're any developer, junior or otherwise, participating in a code review simply means helping that ownership conversion happen however it needs to, whether that means finding and fixing mistakes in the code itself, bringing your own understanding of the code up to speed, or helping to clarify and unify the strategies and logic represented in the code with what will work for the team, which includes you.

thanksforfish 5y ago

I saw a thread recently that suggested reviewers should be running the code, not just reading it. Does anyone here run code as well? If so, I'd be interested in hearing about the experience. I've almost always just read the code and "ran it in my head".

mattm 5y ago
I will do this sometimes if I don't understand how something works and want to confirm it on my own before asking about it. I'll also run the code if I suspect something was missed and want to confirm before bringing it up.
silveraxe93 5y ago
You need to be careful with that. The brain is amazing at reading what you _expect_, not what's actually written. It's the same as proofreading. Your brain just papers over repeated words or typos, or off-by-one errors. Obviously there's a middle ground and it's unreasonable to ask someone to run every single line of code, but it's good to be aware of that.

werber 5y ago

The ask questions section is so important, and not just for Junior devs. I used to be afraid of asking when I wasn’t sure I understood and I’ve learned so much about how to do my job and how people think from always asking now. I wish I hadn’t been so arrogant and lacking confidence earlier in my career

thdc 5y ago

> why they added that piece of logic in that file instead of further downstream This is the kind of thing that has always bothered me about code reviews. I'm confident that my teammates can write working code or any (obvious) bugs can be caught through code review, but is it possible there is a better solution by modifying code somewhere else? For example, there's a bug that is causing several issues, and there is a report for one of those issues. A patch is submitted to fix the issue without fixing the root cause and you're reviewing it. You verify the code looks good and that the issue is fixed - not knowing that the root cause itself is not fixed due to unfamiliarity with the system and it not being anywhere in the patch - and approve it. How much time should be spent looking at whats not in the patch for alternate solutions? How would I know if something could be updated "downstream"? Personally, I used to do it but not so much anymore.

MeinBlutIstBlau 5y ago

My issue with learning codebases is when there is no documentation on what can be passed through as variables in an API. I mean it's fantastic my coworker wrote something to simplify REST retrievals for us, but what format is "string oldDate" in? Is it YYYY-mm-DD? mm/DD? I really hate learning APIs if there is no example of how it can be used. Microsoft has done a stellar job on their documentation but it still isn't enough at times. The "just do it" or "it just works" mentality with code is the #1 thing I hate when using other people's code. Half the time I have to read stuff directly out of their own codebases and actually run it to even figure it out. All because they were too lazy to write a comment somewhere to say "an example of this variable is: ...". In theory, I should be able to use your code right away, not pick at it for hours and go "ohhhh now I see."

rcfaj7obqrkayhn 5y ago

Good advice, but maybe it's hard for people to follow, especially in companies/orgs where people focus on their own promotions so hoarding knowledge = job security