-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Docs: Update PR workflow documentation #10532
Conversation
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.
lgtm, thanks @alamb
I'm thinking should describe a draft status usage as well?
My vision the PR should be in draft in some scenarios:
- Waiting too long(how long?) for the author response on feedbacks
- if the PR is POC, WIP, or other conditions when PR is not really to be reviewed
Took a look. I think this only happens in rare case? The contributor is not only a new contributor to DataFusion but looks like has not participated in open source before (since the GitHub account is created recently). For such cases, I wonder if they will look at the document added here firstly. 😄 |
I plan to incorporate the feedback on this PR, I just haven't had a chance yet. I hope to do so over the next few days |
This is an excellent point @viirya -- I realize some people may not read the doc, but at least now we will have a place to point them |
Co-authored-by: Oleks V <[email protected]>
@comphead -- in so far that we know how we use the draft status, that might be helpful. However, I am not sure it is consistently used enough to explain it. |
* Minor: Update PR workflow documentation * prettier * Clarify leaving PRs open * Update docs/source/contributor-guide/index.md Co-authored-by: Oleks V <[email protected]> * Update docs/source/contributor-guide/index.md Co-authored-by: Oleks V <[email protected]> * prettier * pretty --------- Co-authored-by: Oleks V <[email protected]>
Which issue does this PR close?
Part of #7013
Rationale for this change
I realize I often wait some time between approve and merge which can be confusing and is not really documented. Most recently this came up with @shanretoo on #10476 at #10476 (comment)
What changes are included in this PR?
Clarify documentation explaining the PR cycle
Are these changes tested?
Are there any user-facing changes?