Hacker News Reader

Comment by Someone1234 | Hacker News Reader

I'm not following this. PRs are the first time your reviewers have seen that change, so there is no opportunity downstream to do the things you're suggesting.

You're essentially suggesting pre-PRs, but it is circular, since those same pre-PRs would have the same criticism.

PRs are about isolated core changes, not feature or system design. They answer how not why.

View on HN

> You're essentially suggesting pre-PRs, but it us circular, since those same pre-PRs would have the same criticism.

Walking this road to the end you get pair programming.

You get to design committees where everything has to be approved in advance.

Yep, where productivity goes to die and your developers feel no autonomy/trust.

Usually by the time a PR has been submitted it's too late to dig into aspects of the change that come from a poor understanding of the task at hand without throwing out the PR and creating rework.

So it's helpful to shift left on that and discuss how you intend to approach the solution. Especially for people who are new to the codebase or unfamiliar with the language and, thanks to AI, show little interest in learning.

Obviously not for every situation, but time can be saved by talking something through before YOLOing a bad PR.

Yes, it should be cheap to throw out any individual PR and rewrite it from scratch. Your first draft of a problem is almost never the one you want to submit anyway. The actual writing of the code should never be the most complicated step in any individual PR. It should always be the time spent thinking about the problem and the solution space. Sometimes you can do a lot of that work before the ticket, if you're very familiar with the codebase and the problem space, but for most novel problems, you're going to need to have your hands on the problem itself to get your most productive understanding of them.

I'm not saying it's not important to discuss how you intend to approach the solution ahead of time, but I am saying a lot about any non-trivial problem you're solving can only be discovered by attempting to solve it. Put another way: the best code I write is always my second draft at any given ticket.

More micromanaging of your team's tickets and plans is not going to save you from team members who "show little interest in learning". The fact that your team is "YOLOing a bad PR" is the fundamental culture issue, and that's not one you can solve by adding more process.

I'm not sure what approach you're suggesting?

Asking a more junior developer or someone who "show little interest in learning" to discuss their approach with you before they've spent too much time on the problem, especially if you expect them to take the wrong approach seems like the right way to do things.

Throwing out a PR of someone who doesn't expect it would be quite unpleasant, especially coming from someone more senior.

This is how I try to approach it. I don't think it's a new thing for a new hire to come in hot and try to figure things out themselves rather than spending time with the team. Or getting lost down rabbit holes.

I don't disagree that a practical spike is a good way to grasp a novel problem (or work with a lack of internal knowledge because it's legacy code) but there is still something to be said for attempting to work things out in the abstract too, and not necessarily by adding process, but by redeveloping that internal knowledge and getting familiar with the business domain.

In a greenfield project I will have a lot of patience for a team that doesn't grasp the problem space too well yet, and needs to feel around it by experimenting and prototyping. You have to encourage that or you might not even be building anything innovative.

For the longer term legacy project then the team can't really afford to have people going down rabbit holes and it's more beneficial to approach things in the abstract and reduce the problem as much as possible. Especially with junior or mid-level engineers who can see an old codebase as a goldmine for refactoring if left unattended.

As for the fundamental culture issue... maybe. AI increases the frequency of low quality PRs and puts a bigger burden on the reviewer. I can live with this in the short term if people take lessons from it and keep building up their own skillset. I feel this issue is not unique to my team and LLM-driven development is still novel enough that we're all figuring out the best way to tackle it.

> PRs are the first time your reviewers have seen that change, so there is no opportunity downstream to do the things you're suggesting.

Yes, but I'm arguing for that it shouldn't be the first time they hear about that this change is planned and happening, and their input should have taken into account before the PR is even opened, either upfront/before or early in development. This eliminates so many of the typical PR reviews/comments.

Figure out where you are going before you start going there, instead of trying to course correct after people already walked places.

Reply on news.ycombinator.com