5 ms·

I like your thought process here, but let me propose an alternative. I’m going to assume that we’re talking about Senior->Senior level reviews here. Junior engi

I like your thought process here, but let me propose an alternative. I’m going to assume that we’re talking about Senior->Senior level reviews here. Junior engineers should obviously always get more guidance. But for Seniors, the alternative is: Be extremely picky for PRs from new hires, so that they do things the way that your company does them. Make sure they put files in the right places, and adhere to the “flow” of the rest of the codebase. Also the sorts of style that aren’t picked up by a linter — like architecture choices — should be heavily scrutinized to make sure that they fit with your company’s paradigms. After a while, they’re fully assimilated and you have a coherent style so that everyone is on the same page. Future code reviews go quickly because everyone makes similar architecture decisions and you’re never surprised. This also makes code easier to read if it all follows the same subset of patterns. I’m not actually sure which way is better, but I think that both options have benefits.

marcosdumay 3y ago
I've received plenty of negative feedback on the internet for this (and up to now, discarded all), but I really don't think acceptance review is the correct time for guidance. Yes, juniors need guidance. That means you must read their code, and talk to them about it. They also need real-world feedback. They do really need you not managing all of the interaction they have with the real world. Now, what you can not manage is up for a complex risk analysis, but they really need you to get out of the way at some point.
- johnnyanmac 3y ago
  So, when is the right time?
  - marcosdumay 3y ago
    At some time you plan for it. Preferably a calm time, not one they are waiting a decision from you. E.g. before they start implementing a feature is better than after they finish it.
    - deleted 3y ago
      [deleted]
withinboredom 3y ago
> Be extremely picky for PRs from new hires, so that they do things the way that your company does them. I think the word you are looking for is "hazing": > haze (v): force (a new or potential recruit to the military or a university fraternity) to perform strenuous, humiliating, or dangerous tasks. I had this at my current job. I almost quit because it was downright humiliating to get called an "idiot" in so many nice words. I don't do well with hazing... so I brought out quotes from text books, papers, and famous authors to point out how wrong they were.
- notnullorvoid 3y ago
  They are not suggesting hazing. I don't doubt that you experienced hazing. Being vigilant with new hires to assure assignment on code quality and design is not hazing though. If someone has legitimate concerns about the design decisions made, then they should voice them. However if they are refusing to adhere to guidelines, simply because they dislike the approach then that's being overly problematic.
  - withinboredom 3y ago
    It’s literally the definition of hazing. But instead of being asked to jump in a pool, naked, while snowing, you are asked to build things a new hire has no business building. Then nit-picked for not knowing things. Literally set up for failure. A better solution is to actually sit with them while they build a feature, show them around the code, and answer questions. You know, treat them like a team member instead of making them prove their mettle.
    - BytesAndGears 3y ago
      I definitely didn’t mean it like hazing. I don’t see what’s so wrong about saying “you used inheritance for this relationship, but we have a pattern of keeping classes like this separate, since this system tends to change frequently. Please organize this like xyz module instead.” Just a random example of something that a new person might do who is unfamiliar with xyz module and the complications there. I agree that it’s good to mentor someone new, but honestly I think they still make most decisions themselves, and sometimes those decisions don’t match established patterns that they don’t know about. Ideally you notice sooner than a PR but I also think that people get busy and it’s ok to not have time to monitor everything a new person does. So sometimes it comes down to the code review to notice. It’s not derogatory or hurtful, literally at all, it’s just pointing on that they did something in a way that goes against established patterns, and it’s teaching them what those patterns are.

marcosdumay 3y ago

I've received plenty of negative feedback on the internet for this (and up to now, discarded all), but I really don't think acceptance review is the correct time for guidance. Yes, juniors need guidance. That means you must read their code, and talk to them about it. They also need real-world feedback. They do really need you not managing all of the interaction they have with the real world. Now, what you can not manage is up for a complex risk analysis, but they really need you to get out of the way at some point.

johnnyanmac 3y ago
So, when is the right time?
- marcosdumay 3y ago
  At some time you plan for it. Preferably a calm time, not one they are waiting a decision from you. E.g. before they start implementing a feature is better than after they finish it.
  - deleted 3y ago
    [deleted]

withinboredom 3y ago

> Be extremely picky for PRs from new hires, so that they do things the way that your company does them. I think the word you are looking for is "hazing": > haze (v): force (a new or potential recruit to the military or a university fraternity) to perform strenuous, humiliating, or dangerous tasks. I had this at my current job. I almost quit because it was downright humiliating to get called an "idiot" in so many nice words. I don't do well with hazing... so I brought out quotes from text books, papers, and famous authors to point out how wrong they were.

notnullorvoid 3y ago
They are not suggesting hazing. I don't doubt that you experienced hazing. Being vigilant with new hires to assure assignment on code quality and design is not hazing though. If someone has legitimate concerns about the design decisions made, then they should voice them. However if they are refusing to adhere to guidelines, simply because they dislike the approach then that's being overly problematic.
- withinboredom 3y ago
  It’s literally the definition of hazing. But instead of being asked to jump in a pool, naked, while snowing, you are asked to build things a new hire has no business building. Then nit-picked for not knowing things. Literally set up for failure. A better solution is to actually sit with them while they build a feature, show them around the code, and answer questions. You know, treat them like a team member instead of making them prove their mettle.
  - BytesAndGears 3y ago
    I definitely didn’t mean it like hazing. I don’t see what’s so wrong about saying “you used inheritance for this relationship, but we have a pattern of keeping classes like this separate, since this system tends to change frequently. Please organize this like xyz module instead.” Just a random example of something that a new person might do who is unfamiliar with xyz module and the complications there. I agree that it’s good to mentor someone new, but honestly I think they still make most decisions themselves, and sometimes those decisions don’t match established patterns that they don’t know about. Ideally you notice sooner than a PR but I also think that people get busy and it’s ok to not have time to monitor everything a new person does. So sometimes it comes down to the code review to notice. It’s not derogatory or hurtful, literally at all, it’s just pointing on that they did something in a way that goes against established patterns, and it’s teaching them what those patterns are.