logoalt Hacker News

robgibbonsyesterday at 3:18 PM9 repliesview on HN

For what it's worth, writing good PRs applies in more cases than just AI generated contributions. In my PR descriptions, I usually start by describing how things currently work, then a summary of what needs to change, and why. Then I go on to describe what exactly is changing with the PR. This high level summary serves to educate the reviewer, and acts as a historical record in the git log for the benefit of those who come after you.

From there, I include explicit steps for how to test, including manual testing, and unit test/E2E test commands. If it's something visual, I try to include at least a screenshot, or sometimes even a brief screen capture demonstrating the feature.

Really go out of your way to make the reviewer's life easier. One benefit of doing all of this is that in most cases, the reviewer won't need to reach out to ask simple questions. This also helps to enable more asynchronous workflows, or distributed teams in different time zones.


Replies

Hovertruckyesterday at 3:28 PM

Also, take a moment to review your own change before asking someone else to. You can save them the trouble of finding your typos or that test logging that you meant to remove before pushing.

To be fair, copilot review is actually alright at catching these sorts of things. It remains a nice courtesy to extend to your reviewer.

show 1 reply
phitoyesterday at 4:04 PM

I often write PR descriptions, in which I write a short explanation and try to anticipate some comments I might get. Well, every time I do, I will still get those exact comments because nobody bothers reading the description.

Not to say you shouldn't write descriptions, I will keep doing it because it's my job. But a lot of people just don't care enough or are too distracted to read them.

show 4 replies
simonwyesterday at 3:24 PM

100%. There's no difference at all in my mind between an AI-assisted PR and a regular PR: in both cases they should include proof that the change works and that the author has put the work in to test it.

show 2 replies
brooke2kyesterday at 11:18 PM

I used to be much more descriptive along these lines with my PRs, but what I realized is that nobody reads the descriptions, and then drops questions that are directly answered in the description anyways.

I've found that this gets worse the longer the description is, and that a couple bullet points of the most important things gets the information across much better.

reactordevyesterday at 3:59 PM

I do this too with our PR templates. They have the ticket/issue/story number, the description of the ask (you can copy pasta from ticket). Current state of affairs. Proposed changes. Post state of affairs. Mood gif.

bob1029yesterday at 5:06 PM

> I try to include at least a screenshot

This is ~mandatory for me. Even if what I am working on is non-visual. I will take a screenshot of a new type in my IDE and put a red box around it. This conveys the focus of my attention and other important aspects of the work effort.

show 1 reply
Swannietoday at 1:03 AM

If only there were community standards for this...

Oh, there are, for years :D This has really stood the test of time:

https://rfc.zeromq.org/spec/42/#24-development-process

And its rationale is well explained too:

https://hintjens.gitbooks.io/social-architecture/content/cha...

Saddened by realizing that Pieter would have had amazing things to say about AI.

toomuchtodoyesterday at 3:19 PM

This is how PRs should be, but rarely are (in my experience as a reviewer, ymmv, n=1). Keep on keepin' on.