doc: create ai-guidelines and include to CONTRIBUTING - #62105
Conversation
|
Review requested:
|
|
There may be some ideas we can borrow from https://llvm.org/docs/AIToolPolicy.html - for example "good first issue" should not be picked up by AI is a good one. |
I took inspiration from https://github.com/zulip/zulip/blob/main/CONTRIBUTING.md#ai-use-policy-and-guidelines |
| * **Verify accuracy** of any LLM-generated content before including it in a | ||
| PR description or comment. | ||
| * **Complete pull request templates fully** rather than replacing them with | ||
| LLM-generated summaries. |
There was a problem hiding this comment.
Do we have a template? I thought those are for issues, not PRs.
There was a problem hiding this comment.
Not strictly a template: https://github.com/nodejs/node/blob/main/.github/PULL_REQUEST_TEMPLATE.md?plain=1
There was a problem hiding this comment.
It's not possible to fulfil the instructions "Complete pull request templates fully" based on the contents of https://github.com/nodejs/node/blob/main/.github/PULL_REQUEST_TEMPLATE.md?plain=1 so it looks like this sentence needs to be removed.
There was a problem hiding this comment.
I'd be against this contribution policy update. While many different opinions exist on the licensing terms of the code produced by LLMs, my opinion is that the generated code isn't explicitly licensed and attributed to the original authors so it cannot be considered open source regardless of the used prompt.
| * **Verify accuracy** of any LLM-generated content before including it in a | ||
| PR description or comment. | ||
| * **Complete pull request templates fully** rather than replacing them with | ||
| LLM-generated summaries. |
There was a problem hiding this comment.
| * **Verify accuracy** of any LLM-generated content before including it in a | |
| PR description or comment. | |
| * **Complete pull request templates fully** rather than replacing them with | |
| LLM-generated summaries. | |
| * **Verify accuracy** of any LLM-generated content before including it in a | |
| PR description or comment. |
|
In #61478 (comment) , regarding the usage of Claude Code, @mcollina suggested:
I added it to the TSC agenda tomorrow for awareness/context collection before moving to a proper vote. @indutny sorry about the short notice since this is just one day ahead of the meeting, but if you'd like to join the meeting to present your points please let us know. |
mcollina
left a comment
There was a problem hiding this comment.
lgtm with a sentence removed
|
@joyeecheung thanks for considering me for this! I'd be happy to join if the time isn't in conflict with my work meetings tomorrow. Could you send me an invite, please? |
|
nodejs/TSC#1830 this is the issue. It might be reschedueld to the next meeting if there is low attedance given the timezone. I think we can schedule the discussion for April 1st which will be in the morning PT so a lot of people can join, and I would table the vote for that session. I would try to get an answer from the Board by then. |
|
Discussed at the last @nodejs/tsc. |
84a3b2d to
b696334
Compare
Co-authored-by: Beth Griggs <bethanyngriggs@gmail.com> Co-authored-by: Aditi <62544124+Aditi-1400@users.noreply.github.com> Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com> Co-authored-by: Tobias Nießen <tniessen@tnie.de> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com> Co-authored-by: Efe <dogukankrskl@gmail.com> Co-authored-by: James M Snell <jasnell@gmail.com> Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
b696334 to
2c97efd
Compare
|
I have squashed the commits and solved the linting issue. Nothing has changed. PTAL. |
Per Node.js guidelines, after 2 weeks or more without a response, we consider the objection dismissed.
joyeecheung
left a comment
There was a problem hiding this comment.
Some wording tightening and adding references..
Also I believe we should reference the large PR policy added by #62829 in this document, though not exactly sure where?
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
c095ce0 to
2d77889
Compare
|
Linting seems to be failing, that said it does look like this PR is ready? |
joyeecheung
left a comment
There was a problem hiding this comment.
LGTM with one final wording suggestions, thanks!
| the correctness of new tests against the expected behavior of the feature | ||
| being tested, independently of the feature's implementation. |
There was a problem hiding this comment.
| the correctness of new tests against the expected behavior of the feature | |
| being tested, independently of the feature's implementation. | |
| the correctness of new tests against the intended behavior of the feature | |
| being tested, independently of how the implementation happens to behave. |
I was just reading a post by a maintainer of another prominent OSS project today who was frustrated by a 10K+ LOC AI-generated PR and saw some wording about this behavior that we can borrow - I also saw this behavior when reviewing #63115 which exposed internalModuleStat by mistake and then went on to make even more mistakes by ensuring the internal API is publicly usable 🤦🏻♀️, seems like a common issue of AI-generated big patches
As discussed in today's TSC meeting.
cc: @nodejs/tsc @BridgeAR