GH-15249: [Documentation] Add PR template #15250
Conversation
|
|
Co-authored-by: David Li <[email protected]>
|
Benchmark runs are scheduled for baseline = 7e02fde and contender = 33d677c. 33d677c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
* Closes: apache#15249 Lead-authored-by: Jacob Wujciak-Jens <[email protected]> Co-authored-by: David Li <[email protected]> Signed-off-by: Yibo Cai <[email protected]>
|
My sincere apologies for not getting a chance to provide feedback on this pull request while it was still open. The pull request template looks great! This should help further lower the barrier to entry for contributions. Thank you @assignUser for taking the initiative to make this a reality! |
| @@ -0,0 +1,62 @@ | |||
| # Which issue does this PR close? | |||
There was a problem hiding this comment.
If we start to use this, that will get duplicated with the * Closes .. that the bot adds. Do we then want to remove the bot action? (or some way to unify those? but since this is free form, it might be difficult to have the bot check it)
There was a problem hiding this comment.
That's also an option. As long as we still require an issue and ask it to be in the title, then this is indeed a bit redundant (and we could have a hidden comment explaining that it should be in the title)
| --> | ||
| Closes # | ||
|
|
||
| # Rationale for this change |
There was a problem hiding this comment.
Seeing some first PRs with this, shall we use a smaller header? (eg ### instead of #)
There was a problem hiding this comment.
Current header does looks too big IMO.
|
While I think it is a good effort to improve the PR template and top posts in PRs, my feeling is that this is at odds with how we currently use the body of the top post for the merged commit message. For example, many people will leave the commented-out help messages (since they are not visible anyway), but they will appear in the commit messages. Personally, I think we can move the merge script to only use the title of the PR as commit message, and not the body text? |
|
@jorisvandenbossche - that's a great point about the comments being included in the commit message. Personally, I like the new template (especially how the comments help guide contributors on what to write). However, I agree that the merge script including comments in the commit message is not ideal. Could we have the the merge script remove the comments automatically? Also, I think having the pull request description (as opposed to just the title) in the commit message is preferable because that keeps the rationale for the changes and associated implementation info directly in the Git history for future reference. |
|
@jorisvandenbossche @kevingurney @cyb70289 please see #33620 |
6 participants
Uh oh!
There was an error while loading. Please reload this page.