top | item 41481658

(no title)

EB66 | 1 year ago

This is a really interesting phenomenon that I've experienced before myself but I hadn't fully understood or appreciated as clearly as the author has.

At my company we put a big emphasis on code reviews. We encourage devs to pull request code fairly regularly to keep PRs relatively small (when possible) -- before so much code has been written that it's not really possible to change course without blowing up deadlines. We encourage our junior devs (who might not be capable of identifying bugs or proposing fixes on code written by a senior) to ask questions in their code reviews -- to verify assumptions, to request an explanation of how something works, confirm that a particular edge case has already been considered, etc. It can be hard to get a junior dev comfortable with doing this (questioning a senior dev), but even if the junior isn't identifying bugs it will often lead the senior to better understand their own code and the architectural concepts that underpin their own coding decisions. Like the author points out, this only happens because the senior dev endeavors to explain their work to the junior dev (Protege effect). Also, a good many times it leads the senior dev to re-consider how they wrote something and they might add a revision to address a possible edge case not previously considered. I hadn't thought of it this way before, but this is the Socratic method that the author talks about.

We also put a big emphasis on in-code comment writing -- largely following the commenting principles laid out by John Ousterhout's "A Philosophy of Software Design". These comments are of course for long-term maintenance purposes, but they also benefit team learning. Class, method and variable naming are obviously important too. Our internal code reviewing mantra is that 'I want to be able to read your code like a story book -- when I get to the end, I at least want to be able to understand what happened'. Not always possible, but a good goal. Writing comments and choosing class/method/variable names in pursuit of that goal massively contributes to the learning of the team. During our code reviews, one of the most common requests by reviewers is for the author to add a comment explaining something that was very difficult (or impossible) for the reviewer to grasp on a first read.

This approach has worked very well for us. Everyone learns and our product quality improves.

discuss

order

softwaredoug|1 year ago

You have to be OK getting rid of productive assholes that mess up this culture. The hint of a senior not being open to criticism will instantly halt any ability for a team to have a safe, open dialog.

robingchan|1 year ago

i’ve worked with people like this - big egos were like a bag of lead the entire team had to carry.

EB66|1 year ago

Yeah, very true, when we're hiring we'll sometimes skip over the most talented candidate and pick a candidate who we feel would be the better communicator, easier to work with and fit in with the team. Easier said than done, but we at least try to hire for that.

pipes|1 year ago

This thing with small PRs bothers me somehow. Where does design happen? Do we just make endless tactical changes?

For juniors this is even worse, they get to wait until PR time to validate their design. With trunk based development this means continually commiting unreachable code with back and forth with more senior staff via PRs, instead of sitting down for an in depth discussion where they'd actually learn how to do this job.

This isn't really aimed at the parent poster, just an observation. I've been working since 2006, GitHub brought in PRs and I think we lost something over night: code reviews where you get to sit with more senior staff and discuss your code.

fishtoaster|1 year ago

I've been working through PRs since around 2011 and I don't think I've ever seen a place where PRs were intentionally used to handle design issues. Occasionally design problems will get brought up in PR, but that's always been considered a failure of earlier planning when it happens.

Depending on the company, the design discussion you're talking about has happened:

1. In verbal discussions with other devs before fingers ever touch keyboards

2. In long-form writing (Text docs, then Google docs, then later Notion)

3. In per-project slack channels

4. Out loud, on a whiteboard.

Right now it's very much #4 because my company is 2 devs and a founder. But even here, I wouldn't expect to get anything non-trivial into pull request before figuring out the overall structure with the other dev verbally or in writing.

Sakos|1 year ago

At my first job a few years back I sat down with a senior dev at least once a week (informal meeting) to discuss my tasks and the code bases we were responsible for in general. It was incredibly helpful. I could also request feedback at any time. At the end of it, the final PR might be large, but it had mostly been discussed and reviewed by then, so doing a final review was more of a formality, dotting the i's and crossing the t's to ensure the criteria had been properly fulfilled.

I thought it was a great process.

sojournerc|1 year ago

In addition, writing code that is accessible to juniors (which should apply to a large majority of a code base) increases the velocity of everyone on the team. Readability, modularity, and consistency create standards and examples for a junior to work by, which is a great way to learn and lower the risk of bugs and surprises.

ben_w|1 year ago

Yup. This is why I favour simple architectures like MVC over complex ones like VIPER.

While there may be exceptions, I default to: if an app is so messy as to consider a more complex architecture, it probably has so much going on as to be difficult for the end user to use it.

doctorpangloss|1 year ago

> ...code reviews

> ...PRs relatively small (when possible)

> ...deadlines

Truth is if you are doing the kind of work that fits into code reviews, small PRs and neat deadlines, does any of it matter? You're describing a tech enabled agency, not a startup. Sure customers have alternatives, but in the same sense that we have choices when booking airline tickets: you are describing a development process for a product that is fundamentally fungible, so the main differentiator among producers is costs, not management strategy.

eximius|1 year ago

Why on earth would a startup's code not be reviewable?

If I'm doing a big chunk of work, I still do it in small PRs. That first PR (or a design doc) might outline a strategy for the ones that follow, but still try to have small PRs. Or, worst case, I do a big chunk of work and break it out into small logical chunks for review.

And even startups can have deadlines.