Update PR template to mention coding assistant attribution#9451
Conversation
neilcsmith-net
left a comment
There was a problem hiding this comment.
Looks good, thanks! Hadn't taken in you planned to change the checker too, but makes sense.
| - Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field. | ||
| - LLM assisted commits should be attributed with a `Assisted-by: MODEL_NAME MODEL_VERSION` line appended to the commit message. | ||
| - Please mention coding assistance in the PR description too (eg. by adding the same `Assisted-by` line from above) | ||
| - Please describe the changes in your own words focusing on the important aspects |
There was a problem hiding this comment.
I still think this should emphasise the human side, that contributions without understanding are not helpful - eg. "Please describe the changes in your own words - we'd like to know you understand the changes being made!"
There was a problem hiding this comment.
I agree with that. At the very least the PR description should be human-written, for proving understanding of the changes, and to provide some signal to reviewers about the author's commitment to see the PR through.
Neil's wording would work, I think. (Including the exclamation mark.)
There was a problem hiding this comment.
what if we add another point in the sense of:
- Contributors are expected to review and understand the submitted code, verify that the change is correct, necessary, and consistent with project standards
If a contributor can not explain a change the contribution may be rejected.
but we would have to change the title of the section a bit since this goes beyond PR text.
There was a problem hiding this comment.
something for the first list maybe? L11-14
There was a problem hiding this comment.
I added the updated template to this PR description to be able to see the changes easier.
There was a problem hiding this comment.
I'd rather keep it shorter and friendlier. This is also, after all, primarily text for humans to read. I know I can be guilty of being too verbose sometimes 😄 But the less there is to read the more likely it is people will take in what they need.
There was a problem hiding this comment.
I have the tendency to like reading/stating requirements. I can see that this sounds friendlier. updated
|
LGTM too |
|
there is also the problem that CLI created PRs completely ignore the template and won't see any of this. But lets start somewhere. One step at a time. |
- LLM/coding assistant attribution in commits and PR description via 'Assisted-by' tag - updated paperwork check
minimal LLM guidelines for contributors
see https://lists.apache.org/thread/qhr3mfxg5wsv89d80nxzdd63ldjth07p
^Add meaningful description above
Click to collapse/expand PR instructions
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
LLMs, Commit messages and PR description:
git log) that all commits have a valid name and email address for you in the Author field.Assisted-by: MODEL_NAME MODEL_VERSIONline appended to the commit message.Assisted-byline from above)If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)