PR #926 situation, need guidance #927
Replies: 2 comments 8 replies
-
|
Thanks for bringing this up. I noticed that PR had an AI smell. Plone has struggled with AI abuse for years. Google Summer of Code (GSoC), Hacktoberfest, and other "pad your CV at the expense of open source software volunteers" events exploits the goodwill of FOSS community members without giving back. We have professors telling their students to submit pull requests for homework, and they don't even use the software! In Volto, we even had one person from Asia submit a PR using AI that copied another contributor's unmerged PR verbatim, and it was a Swedish translation file. The original contributor was furious, and the plagiarist pleaded it was AI that did it and that he wasn't responsible for wielding the tool. The flood of slop and garbage from first-timers into Volto resulted in an update to its contributing policy, and Plone will soon publish an AI policy. I drafted this AI policy for Plone, but I'm afraid it will be watered down. As I result, I'm always skeptical of first-timers, and especially when it has an AI smell to it. |
Beta Was this translation helpful? Give feedback.
-
|
I would say: Hacktoberfest is nice for people to learn.
We are free to experiment with this. Suggestion for a process:
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
@niccokunzmann @stevepiercy
Need your input on PR #926.
I created Issue #925 asking for tests on Conference parameter edge cases. Added a comment explicitly saying "Wait for PR #921 to merge first" because the tests would need to be written against the fixed code.
PR #926 was opened 4 hours later, 6 hours before #921 merged. They didn't wait.
The problem: their tests expect the wrong behavior.
Their code doesn't have the normalize() function from PR #921. It's still the old code that passes lists through unchanged. So their tests expect:
But after #921 merged, it returns:
Their tests pass because they're testing against pre-#921 code. If we merge this, it reverts the normalization fix.
They also test invalid types like feature=0 and feature=False that can't pass the type hints (feature: list[str] | str | None).
The PR body reads like it was AI-generated with lots of expansion on my original issue. The code has the same feel.
Here's what bothers me: this feels like autopiloting. Issue created, fed to an LLM, PR generated within hours. They posted "Ready for review!" 14 seconds after the first CI test passed. No time to actually review test results, no testing against actual main branch, no verification the tests make sense. Just fast output.
I use AI tools myself (Copilot for commit messages, code suggestions, boilerblate). That's fine. But this looks like someone skipped the thinking part entirely. Didn't read the instructions, didn't test the code, didn't understand what #921 changed.
We might need a policy on this. Not banning AI, but making it clear that instructions in issues must be followed, code must be tested against current main, and you need to understand what you're submitting, not just generate it.
What should I do here? Close it with explanation? The instructions weren't followed and the tests are fundamentally wrong. I'm new to reviews so looking for guidance on the right approach.
Beta Was this translation helpful? Give feedback.
All reactions