MNT/DOC Autoclose schedule doesn't run on forks and improved structuring #32889
Conversation
lucyleeow
left a comment
There was a problem hiding this comment.
Overall I think we can be more succinct and structured in the autoclose comment. I am not sure repeating ourselves is going to stop the more egregious offenders and the longer we make this, the less likely people are going to read it.
What about an overall structure of:
- Paragraph 1: PR does not standards (I think it's fine currently)
- Paragraph 2: Link to ways to improve PR. Advise they are responsible for driving this PR. If they do not know how to improve their PR, suggest building foundation with other contribution types (link to new contributor section) or asking questions after doing research, which someone from community may respond to.
- Paragraph 3: If they improve the PR, the label may be removed. Advise against them requesting label be removed or ping-ing maintainers (I think you did not want to tell people not to ping maintainers, but I think its worth having, they can still comment and ask questions, but don't ping maintainers)
I've made more specific suggestions below.
| * ensure to only submit code you can explain and maintain, as you might need to defend | ||
| and discuss it during review, clarify your reasons, understand the use case, do | ||
| additional research, or investigate alternatives, | ||
| * inspect and solve tests failing locally or in the CI. |
There was a problem hiding this comment.
I think this is already mentioned in the PR checklist
| :ref:`stalled_pull_request` and :ref:`stalled_unclaimed_issues`, | ||
| * fill in all sections of the issue or pull request template provided by GitHub, | ||
| including a clear description of the problem or motivation, and any relevant code | ||
| examples, |
There was a problem hiding this comment.
I though the original was mostly adequate? I am reluctant to make it wordier, but may we could add that they should explain their through process for PRs...
| examples, | |
| * complete all sections of the issue or pull request template, | |
| including a clear and concise description of the issue or motivation and | |
| thought process behind the pull request |
| * ensure the changes are minimal and directly relevant to the described issue, | ||
| * ensure your PR complies with scikit-learn's development conventions (which you can | ||
| learn about from the docs and by going through similar prior PRs), | ||
| * ensure to only submit code you can explain and maintain, as you might need to defend |
There was a problem hiding this comment.
I think this may be included in the AI contributions section and honestly I am not sure writing this several times is not going to stop offending users. These people are unlikely to read or abide by this no matter how many times we write it...
| * ensure your PR complies with scikit-learn's development conventions (which you can | ||
| learn about from the docs and by going through similar prior PRs), |
There was a problem hiding this comment.
I feel like this is a bit vague and I think the PR checklist already does a reasonable job of discussing some of these conventions: https://scikit-learn.org/dev/developers/contributing.html#pull-request-checklist
| maintainer suggestions, and doing additional research if requested. If all | ||
| that sounds demanding, that's because it is, and if you are a [new |
There was a problem hiding this comment.
Not sure about how this phrase comes off: "if all that sounds demanding, that's because it is,"
| clarifications and changes. Disclose any AI assistance per our | ||
| [Automated Contributions Policy](https://scikit-learn.org/dev/developers/contributing.html#automated-contributions-policy). | ||
| Scikit-learn maintainers cannot provide one-to-one guidance on this PR. | ||
| However, as a community that values helpful discussions, we encourage you to |
There was a problem hiding this comment.
This seems vague - helpful to whom? And I am worried someone will use this against us, because we don't have time to engage in discussions.
Reference Issues/PRs
What does this implement/fix? Explain your changes.
This PR fixes the
autoclose-scheduleworkflow to not run on forks, links to a central list of how to improve a PR, and implements some changes in the wording. Specifically, it makes it more clear to new contributors where the autoclose label was set, that they are responsible to fix the issues and advises them to not request the label getting removed.AI usage disclosure
I used AI assistance for: