MAINT: Add PR template content check action#32082
Conversation
Uh, I do that 😅 but also is probably good if it also yells at maintainers. Thanks for doing this! |
Hmmm, I'm not sure how I feel about that. I certainly sometimes ditch the template if my change is quick and obvious. Also I know @timhoffm often doesn't use it. I see the argument that we should model the behaviour we expect from other contributors. On the other hand, the rationale for insisting on an AI declaration and "make sure you can explain this" is so we can gauge whether the author is competent to discuss the change with us. I have never had any doubts about Tim's competence! |
This probably points to places where the template can be simplified, but maybe something like %X filled or optional fields? |
|
Alternatively (and maybe simpler) we can check for author membership to the org - we'd only run it for non-members of the matplotlib org. |
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
There doesn't seem to be any need for the full history.
| - name: Read PR Template | ||
| id: template | ||
| uses: juliangruber/read-file-action@271ff311a4947af354c6abcd696a306553b9ec18 # v1.1.8 | ||
| with: | ||
| path: ./.github/PULL_REQUEST_TEMPLATE.md | ||
| # This step is required because the PR body is CRLF (Windows-style line ending) | ||
| - name: Clean PR content | ||
| shell: bash | ||
| id: clean | ||
| env: | ||
| PR_BODY: ${{ github.event.pull_request.body }} | ||
| run: | | ||
| { | ||
| echo 'cleaned<<EOF' | ||
| echo "${PR_BODY}" | tr -d '\r' | ||
| echo EOF | ||
| } >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
I do not understand why these are separate steps. They could all go in the bash script.
|
T.b.h., I usually ignore the PR template for two reasons:
Possibly, I should advocate for reworking the template, so here we go #32093. |
|
Agreed. I'll go further and say I am not a fan of auto-enforcing templates or forms for things that are often better expressed in freeform. |
|
That's fair. We discussed this check at the last weekly meeting as a quick way to flag potential ai contributions - most of them seem to violate the PR template, and we thought this could be a good signal to spare maintainers' time. If we want to keep this idea, we could:
Otherwise happy to close and explore other ideas 😄 |
PR summary
Here is an idea of an action to check completeness of the PR template. If it is not filled, or any of the sections are deleted from it, the PR adds the
status: autoclose candidatelabel to it. We could add a path to remove the autoclose candidate label if the PR template is corrected, but I think that may be best left to maintainers.Tested it locally with
nektos/act. For example, the action would label #32028 and #32006, and not #32081.Should we do this only on PR open, or on any changes to the PR?
AI Disclosure
Used claude to verify the bash part of the PR.
PR checklist