-
-
Notifications
You must be signed in to change notification settings - Fork 8k
DOC: Clarify draft PR and move from ways to contribute to PR guidelines #30507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There is also a paragraph about draft PRs on the development workflow page. It would be good to link your new explanation from there too. |
doc/devel/pr_guide.rst
Outdated
a result of feedback. Authors should clearly communicate why the PR has draft | ||
status and what needs to be done to make it ready for review. In particular, | ||
they should explicitly ask for specific feedback if needed. By default, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some examples here might be good for expectation setting cause these all seem like 3 rewordings of the same thing to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm reluctant to add examples here. I feel it makes the topic more verbose and does not add much value. These sentences are a nudge on thinking about these related aspects and communicating effectively to reviewers. As indicated by should, It's not a hard and detailed requirement that I want or expect users to lengthily craft. Read these sentences and write down what comes to your mind from them. That is good enough.
Here is what AI says - and it very well captures my intention. - I'm using this to illustrate that it's nowadays easily possible for a user to get additional explanation and examples with out us having to spell out everything excessively.
That guideline is emphasizing transparency and efficiency in the pull request (PR) process. Here’s what it really means in practice:
Communicate why it’s a draft
- Don’t just mark a PR as Draft without context.
- Explain whether it’s a work in progress (e.g., missing tests, incomplete implementation, performance concerns, refactor still underway, etc.).
- Example: “Marking this PR as draft because integration tests are failing locally and I need to resolve that before it’s ready.”
Define what’s left to do
- Give reviewers visibility into what’s missing before it can be promoted for full review.
- Example: “Still need to add error handling for edge cases and write unit tests for the new helper functions.”
Request targeted feedback (if needed)
- Even in draft form, you might want feedback on specific aspects (design choices, API shape, approach to handling dependencies, etc.).
- This helps reviewers focus on what’s most useful at that stage instead of reviewing everything prematurely.
- Example: “At this stage, I’d appreciate feedback on the database schema design before I proceed with writing migrations.”
👉 In short:
A draft PR shouldn’t be a mystery. It’s not just “unfinished”—it’s an opportunity to keep collaborators aligned. Clear communication ensures reviewers know what’s done, what’s not, and where they can help right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we link to one of the guides that AI is pulling from?
Read these sentences and write down what comes to your mind from them.
I think I'm getting caught up on what's in scope for specific feedback, and the AI answer of targeted feedback on specific aspects is a bit clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced: "specific feedback" -> "targeted feedback"
Can we link to one of the guides that AI is pulling from?
I don't understand this comment. How do you come to believe AI is pulling from a specific guide? I've just asked to explain my wording in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you come to believe AI is pulling from a specific guide
Because I'm used to google's version that links to sources. And the writing seemed super cohesive so I was hoping it mostly came from one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was ChatGPT 😛
baad0d4
to
dffff23
Compare
dffff23
to
f791853
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me. Thank you for picking this up @timhoffm.
Closes #30436.