I lead a small team of inexperienced devs who work 8h per week (less in reality). Asking them to review my pull requests means I wait around 2 weeks every time before I can merge my changes. What's the best way to include them without breaking myself down so much?
Posted by blondbrew@reddit | ExperiencedDevs | View on Reddit | 47 comments
LogicRaven_@reddit
I don't think you can effectively mentor and get things done with 8h/week. That is very low amount of hours and just catching up with team context and what is happening will take a significant portion of available time.
If this is really the only way of time allocation, then I would give them well-defined tasks, review their changes, but wouldn't expect them to keep up with code changes done by you or review your code.
Mentoring and proper mutual code reviews can be introduced when time allocation goes up, for example to 60% (3 workdays).
titpetric@reddit
agree, with such low commitments, some sort of "good first issue" bag could be productive, reverse the review in this case, and agree how feedback is handled, timelines, TDD, multiple authors, looking at histort...
For the OP, he needs a pair with the same time commitment as him, maybe in a +4 timezone so there's a relay, or maybe a same city coworker. Obviously give interns their intern stuff to do rather than block his process 🤠+2 or more doesnt make sense unless you always need someone (service level agreements)
cosmopoof@reddit
If you asked them to review it, you have to accept that you've made a suggestion to which they answered with "no, thanks".
A part of leadership is to be clear and make clear your expectations. This means that there should be a clear difference between asking and telling.
If you don't want to make rules, you need to accept it that your proposal was simply not convincing enough.
ivancea@reddit
Honestly, even a senior could have problems reviewing PRs while working only for 2h/day. It's not a "no, thanks", it's a "I don't have time". Specially if there's more people asking for reviews, no idea.
Without more context, feels like an organizational issue, which could indeed hide an expectations issue
Training_Strike3336@reddit
Sure, if your PRs are fucking gigantic.
Small, consumable changes that build towards the goal.
ivancea@reddit
I don't believe you actually worked in a real project and did real in-depth reviews. Not after reading that.
Some PRs, you have to launch them locally to test. Some will require investigation from your side. Most of them, discussions. Unless you work on a toy project, not everything is clear and straightforward.
Training_Strike3336@reddit
We moved to a more frequent commit model.
The changes build into a large change, but individually are small and relatively easy on the eyeballs.
If you work in a monolith, it's probably not possible. But in a micro service setup, it is absolutely possible.
5p0d@reddit
It’s possible in a monolith too, provided you have enough experience in the monolith to know how to pace work and split them into smaller PRs
titpetric@reddit
Another side of this approach are LSCs (large scale code changes). For example, if you can express your change in a semgrep rule, it becomes this weird metaprogramming search and replace. The semgrep would maybe be a 1000bytes per rule, while the change would impact thousands of lines (my personal best is around 20K lines changed). this aids in review, as the review is minimized by several orders of magnitude. Generally these are style changes, api changes,...
Some other things (gomports, gofmt, gofumpt, golangci-lint wsl) are nearly always needed/required, healing some stylistic choices for readability and to consider the release. I think i had to write about 4 monolith scaffolds before i found a good fit, and chances are you joined a team and the codebase is somewhere in the spectrum of those iterations.
Either way, if it's not part of the process, usually people fail even before trying, as priorities are elsewhere. Enterprise is about doing the least possible as by extension the most safe change you can make without fucking up things too badly, to continue getting paid. That culture is usually what kills code quality, rather than having a service and goals based approach. Quick feedback loops dont allow for stew time, and sometimes the smallest most innocent changes have huge implications, particularly around type changes (struct, ptr, interface, casts) and honestly if you merged everything else...
GaTechThomas@reddit
Taking a different angle...
Approach the code in a way that enables more automation so that reviews have fewer types of things to care about.
For example, choose a code style that can easily be auto-formatted. Be sure that automated tests have high code coverage. Design in a way that the minimum amount of code is written and maintained - the "core domain" in a DDD world.
This will get you to a place where the things you are reviewing are mostly around correctness of the design, alignment with team-determined code style, confirmation of acceptance criteria, etc.
BoxyLemon@reddit
How is this post getting 70 upvotes? When I leave out context, I get bashed immediately and downvoted. This person leaves out 80% of intormation and gets tons of upvotes.?
ladycammey@reddit
So here's the thing, with a team of inexperienced devs working that few hours a week (I'm guessing because they're divided a bunch of ways? Or are part-time school folk?) you need to think about what your goals really are here.
Are your goals to mentor/develop the devs, or is it to actually get the project done? Because if it's to get the project done, I think you're idea here is out of place - you can't afford that degree of slow-down for a mentoring task. If your goal is to develop them, then you need to stop worrying about the project's progress and accept that it's going to be slow as a snail.
Frankly, actually reviewing pull-requests (as opposed to just like... skimming and approving them) would probably take them longer to read than it took you to write, depending on the complexity of the project. Obviously they can hit a checkbox - but that's not going to be enough to learn anything besides good manners.
If you really want to try to force this, then I'd book a 30 minute 'review meeting' once weekly in which you present/walk through your code to them and then ask them to ask questions/approve/deny. Likewise make it an hour and then you can do their weekly reviews too. But like, that's already giving away 1/8 your time per week on just reviews - you'll have to decide if that's worth it or not.
Really, you're in a weird situation.
PragmaticBoredom@reddit
The reason for the 8H weeks is some all-important context that the OP left out.
The only thing that comes to mind is that these inexperienced part-time devs are interns, or similar to interns. Hiring them is not an efficient way to finish projects so I would assume the company knows that speed isn’t top priority.
Therefore, training and developing them is likely the goal.
However, the OP’s approach feels very hands-off and async for a team of inexperienced people that are supposed to be learning. You have to approach these people more actively: Sending them a code review and waiting for it to happen in 2 weeks isn’t it. You have to have the code review ready to go and tell them they need to do it when they come in (or sign on) for their 8H. Schedule some time to go through it with them if necessary.
When people are only working 8H per week and your job is to teach them, you need to plan, prepare, and shape those 8H intentionally. Waiting for them to get around to things like any other dev isn’t going to work when they’re inexperienced.
blondbrew@reddit (OP)
I also just work 8h. Everyone is remote. We should be discussing my problem more from a perspective of how to organize work on an open source project, I guess.
PragmaticBoredom@reddit
Your situation is so far from normal that you really need to explain all of these things in the opening post if you want good advice.
You can’t just omit details like the fact that it’s an open source project and everyone works only 8H and expect good advice for your situation. The default assumption is that we’re all discussing traditional jobs unless someone explains they’re not.
behusbwj@reddit
Not just interns. They’re likely unpaid interns. I’ve seen these shenanigans before in startups
xampl9@reddit
Agree that scheduling a meeting for code review is a good idea.
“And this is why I did this thing this way…”
whyamievenherenemore@reddit
I don't want to get into the details BUT you can't wait 2 weeks for a code review. You'd be better off finding one attentive dev to work alongside you OR just removing the need for a reviewer and merging yourself.Â
alternatively... you could sell your code better.. ie: give them a REASON to review the PR, add screenshots that display the new functionality so they're encouraged to review things sooner.
TheOnceAndFutureDoug@reddit
Merge it and say, "This is what I did. If you have anything you feel needs changing or questions let me know and I'll make sure to address them".
It is unrealistic for you to include them in this situation.
Alpheus2@reddit
The obvious solution is for you to stop coding and prioritise reviewing their work.
Once you close all PRs I suggest you have a candid conversation about what makes ongoing changes so bulky or confusing.
IGotSkills@reddit
Yeah this doesn't work if you enjoy authoring code. In the model you propose, you become the only code reviewer
Alpheus2@reddit
Is that worse than waiting 2 weeks for any changes to go forward?
IGotSkills@reddit
Get buyin from management and gamify it!
Assign points to being an approver of a review, you lose 5x those points if it causes a production issue.
Make a public leaderboard. They post the link to the PR they reviewed and you verify
At the end of the month whoever has the most points gets a gift card or something silly.
mrfredngo@reddit
I don't understand. If you're leading them, you should be reviewing their work, not the other way around. They'll learn through your reviews of their work.
trying-to-contribute@reddit
Some paired programming with a choice candidate perhaps? Slowly train one of them up, and then you two can work on training the other folks up together?
Abadabadon@reddit
Periodic scheduled meetings to check in.
If they reviewed the mr, meeting deferred.
If they didn't, tell them they need to review in their own time and that you don't like reviewing code in meetings. Then review the code in the meeting.
They'll get the point.
Radrezzz@reddit
What’s the point of mentoring someone who puts in so little work (8h/week)? Unless they’re busy on some other projects?
PragmaticBoredom@reddit
My guess is part time interns.
Radrezzz@reddit
If that’s the case and they are choosing not to take advantage of the learning opportunity, then why is this OP’s concern?
teerre@reddit
Experienced developers have trouble reviewing code, it's unwise to expect interns to do so and quickly at that.
honor-@reddit
Why are you doing code review if you never get a quality feedback?
zerocoldx911@reddit
I thought it was 8h per day, are these outsourced from India? It sounds like it
Comfortable_Claim774@reddit
The easy solution, if you need those PRs to go through faster: merge them without review, if you can. Speaking from experience, sometimes even in teams full of seniors working 40h/week it can be hard to build a culture of quick/useful PR reviews.
You are most likely producing code at a pace that would take a junior dev their full 8h per week to properly review, so the equation doesn't really make sense.
If you still want the other devs to get experience reviewing code, keep one of the less urgent PRs open for them to review, and merge the others when you need to.
dvali@reddit
Honestly, if they are only working eight hours a week then making them actually care about the work is going to be next to impossible. It's not realistic to expect that of part time staff, let alone quarter time.
Have you considered getting them to do the code work and you the review work? Keep in mind the reviewer is the one who's really deciding to put that code in production. It will be more interesting for them so you might get better productivity and you can think of yourself more as an architect, pulling the pieces together.
I'm not convinced getting juniors to review senior code is a good move at the best of times. They won't be able to spot the stuff that really matters. I understand you wanting to teach them, but it doesn't sound like this is really a mentoring role, and it's not your responsibility to break your back when they aren't doing their half.
jkingsbery@reddit
We need some context on why the other devs work 8h per week.
If they are interns who you are trying to teach, I would not wait for them to review. Instead, if you want to include them, you can either offer to pair program with one of them, or you could schedule a 30 minute walk through of the code. There are other ways of teaching people, and using the same mechanism with them as people who work 40 hour weeks is unlikely to be effective.
Wishitweretru@reddit
Well.. You could run different branches, and make the review your -> Dev -> Reviewed -> Stage -> Prod special new "reviewed" branch as part of your Pre deployment process. That way the work continues, and they are reviewing a larger body of code, helping them get a bigger picture... seems like a little micro-management over kill, but it would serve your purpose.
Comprehensive-Pea812@reddit
I hope you got paid ten times their salary lol
thomas_grimjaw@reddit
That's way too little time on code for the team, especially if they are inexperienced.
If your goal is to mentor them, then schedule a call with everybody and pick one of them to review it on the spot every week.
If your goal is to get a project done, then you're better off just doing it yourself, as this is the worst team setup I've ever heard.
ongamenight@reddit
The best way to include them is to increase their capacity. 8hrs/week is too limited and they'll continue to be inexperienced with the work flow that you're doing the code and they're reviewing.
They should be the one doing the code for that real hands-on experience and you reviewing their code and commenting. Also PRs should atmost only stay stagnant for a week. 2 weeks review is too long unless it's like thousands of changes in a single PR.
ezaquarii_com@reddit
It's a two way traffic. They need to be interested to benefit from your offer.
You bring the water to the horse, but you can't force it to drink it.
Move on with life.
titogruul@reddit
I think it's important to understand the cause of the high latency turn around. It may be different for different engineers too.
Is it an efficiency problem? As in they would like to review but it takes too much time? You can try pair programming/review to understand their challenges a bit better and then coach through that. E.g. one of my team members recently explained that they pull and patch each PR they review in order to validate it works. Yea, no wonder they take a long time to review and that's also not the level of review I'm looking for. ;-)
Is it a being organized problem? Work with your/their manager to first confirm that your expectations are grounded in reality (24hr seems sane to me!) and then make sure they enforce those expectations.
Is it a conflicting priority problem? E.g. they're expected to do 40hr of coding on top of reviews and what not? I'd also engage a bit 1:1 to understand conflicting priorities and then work with management to be clear on what is prioritized and then hopefully that will fix it. One outcome could be is that your PR reviews take a back seat, but at least you're on the same page and get signal if what management would prefer: reduced velocity or increases risk.
I would also recommend picking a person to test the waters. In many examples above the first step is to understand the challenges those folks see and it's best to go deep and limited scope to make sure you understand the challenges well before you jump in with solutions.
Fwiw, I'm dealing with similar situation myself and I think it's a combination of 1+2 plus management trying to fix it without having difficult conversations. My crutch is that I have an engineer who can review things relatively quickly and I'm working on the rest.
neilk@reddit
My first impulse is that you are in one of the rare situations where you should be exempted from mandatory reviews before deploy, but it depends.
Are you doing a regular 40h week and your team does an irregular 8h week? Are you all remote? How many people?
Are you in an organization where reviews are mandatory? Do you own the results (or the company?)
What’s your testing infrastructure and culture like? Can you move with confidence by just relying more on tests? (This is hard with some projects, like video games).
I assume you’re using source control where, if we are disciplined and make small commits, a developer who shows up later can figure out what happened? And you have Slack or some way to announce stuff, like “I’m refactoring the XYZ module, hold on to your butts”
truevignesh@reddit
8hr/week is too short, they can take care of only their work (since you said “inexperience”) But did you try any tool that asks or nag them on your behalf?Try candidteams it gives the option to ask for review until its reviewed and also you can set a goal for you team before which a PR should be first reviewed. There are also features to improve inexperienced dev by guiding them to follow a process that suites your team
neilk@reddit
Cancel reviews.Â
We review code to move faster with greater confidence. In your case reviews can only slow you down.Â
In your case you can’t even meet secondary goals like mentorship or reducing reliance on one person’s knowledge. After a gap of two weeks nobody’s going to retain much, and you can’t reduce reliance on yourself.
If you work a regular high-skill 40h/week (?) and your team has an irregular low-skill 8h/week, you are really a solo dev that can assign low priority and non-critical tasks.
AppropriateRest2815@reddit
I would schedule a half hour per week to walk through all or part of a code review for a ticket. you don't have time to go through all of them but you can handle your mentoring task while getting a little more PR review done.
dystopiadattopia@reddit
Are PRs required before merging? (They should be.)
Maybe you should have "pairs" PR reviews so you're both looking at the code at the same time, and so you can start teaching your inexperienced devs about how to properly review a PR.
Maybe even purposely put in some substandard code to see if they'll notice it and call you out on it.
I don't even like waiting 2 hours for a review, I can't imagine 2 weeks.
BigBoiiInDaCluhb@reddit
The way I've overcome this in several companies is a shared slack channel like
#{team name}-review-requestswhere you post in a link to the PR and maybe some additional context around it.  Make a point of going through it quickly during/after daily so that any long-standing PRs are addressed after max 2 days rather than weeks.