Hacker News Reader

Comment by ab_io | Hacker News Reader

100%. My team started using graphite.dev, which provides AI generated PR descriptions that are so bloated with useless content that I've learned to just ignore them. The issue is they are doing a kind of reverse inference from the code changes to a human-readable description, which doesn't actually capture the intent behind the changes.

View on HN

I tell my team that the diff already perfectly describes what changed. The commits and PR are to convey WHY and in what context and what we learned (or should look out for). Putting the "what" in the thing meant for the "why" is using the tools incorrectly.

Yes, that’s the hard thing about having a “what changed” section in the PR template. I agree with you, but generally put a very condensed summary of what changed to fulfill the PR template expectations. Not the worst compromise

My template:

1. What is this change supposed to do?

2. Why is this change needed?

3. How was it tested?

4. Is there anything else reviewers should know?

5. Link to issue:

There's no "What changed?" because that's the diff. Explain your intent, why you think it's a good idea, how you know you accomplished your intent, and any future work needed or other concerns noticed while making the change. PR descriptions suffer from the same problem as code comments by beginners: they often just describe the "what" when that's obvious from the code, when the "why" is what's needed. So try very hard to avoid doing that.

It's same same issue we had 20 years ago with javadoc. Write what you want to do, not how you do it.

i++; // increment i (by 1)

My PR templates are: - what CONCEPTUALLY changed here and why - a checklist that asserts the author did in fact run their code and the tests and the migrations and other babysitting rules written in blood - explicit lists of database migrations or other changes - explicit lists of cross dependencies - images or video of the change actually working as intended (also patronizing but also because of too many painful failures without it)

Generally small startups after initial pmf. I have no idea how to run a big company and pre pmf Im guilty of "all cowboy, all the time" - YMMV

Does the PR description not end up in the commit history after merge? A description of what changed is very useful when browsing through git logs.

> A description of what changed is very useful when browsing through git logs.

Doing a blame on a file, or just looking at the diff of the pull request gives you that. The why is lost very fast. After a few months it is possible that the people that did the change is not anymore in the company, so nobody to ask why something was done.

"Oh, they changed the algorithm to generate random numbers". I can see that in the code. "Why was it changed?". I have not clue if there is no extra information somewhere else like a change log, pull request description, or in the commit comments.

But all this depends on the company and size of the project. In your situation may be different.

Not just browsing, but also searching.

The PR spec for some open source projects are quite onerous.

What is unspoken here is that some open projects are using cost of submission AND cost of change / contrib as a kind of means of keeping review work down.

Nobody is correct here really. It's just that the bottlenecks have changed and we need to rethink everything.

Changing something small on a very large project is a good test. A user might simply want a new optional argument or something. Now they can do it and PR. But the process is geared towards people who know the project better even if the contributor can run all the tests it is still not trivial to fill in the PR request for a trivial change.

We need to rethink this regime shift a bit.

you mean we will get even more of these sort of useless comments?

  // loop over list and act on items
  for each _, item := range items {
    item.act()
  }

Reply on news.ycombinator.com