Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CONTRIBUTING.md: Add PR closing policy #11596

Merged
merged 6 commits into from
Mar 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ are important to the project's success.
4. Optionally [format and check your stubs](#code-formatting).
5. Optionally [run the tests](tests/README.md).
6. [Submit your changes](#submitting-changes) by opening a pull request.
7. Make sure that all tests in CI are passing.

You can expect a reply within a few days, but please be patient when
it takes a bit longer. For more details, read below.
Expand Down Expand Up @@ -627,6 +628,13 @@ processed incorrectly by a type checker. It is also helpful to add
links to online documentation or to the implementation of the code
you are changing.

As the author of the pull request, it is your responsibility to make
sure all CI tests pass and that any feedback is addressed. The typeshed
maintainers will probably provide some help and may even push changes
to your PR to fix any minor issues, but this is not always possible.
If a PR lingers with unresolved problems for too long, we may close it
([see below](#closing-stale-prs)).

Also, do not squash your commits or use `git commit --amend` after you have submitted a pull request, as this
erases context during review. We will squash commits when the pull request is merged.
This way, your pull request will appear as a single commit in our git history, even
Expand Down Expand Up @@ -690,3 +698,37 @@ When merging pull requests, follow these guidelines:
It should be valid Markdown, be comprehensive, read like a changelog entry,
and assume that the reader has no access to the diff.
* Delete branches for merged PRs (by maintainers pushing to the main repo).

### Marking PRs as "deferred"

We sometimes use the ["deferred" label](https://github.com/python/typeshed/labels/deferred)
to mark PRs and issues that we'd like to accept, but that are blocked by some
external factor. Blockers can include:

- An unambiguous bug in a type checker (i.e., a case where the
type checker is not implementing [the typing spec](https://typing.readthedocs.io/en/latest/spec/index.html)).
- A dependency on a typing PEP that is still under consideration.
- A pending change in a related project, such as stub-uploader.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me of the implementation-pending label. How should that interact with this policy and the "deferred" label?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably we don't really need both labels -- we could probably merge them into one :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have too many labels anyway. They could use a good cleanup. But I agree that merging implementation-pending into deferred seems like an easy first step.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the project and project-discussion labels could also probably be merged

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the idea of "implementation-pending" is that we're specifically waiting for a third-party library we're stubbing to release their implementation. That's maybe a little different from "deferred", but not sure it's worth a separate label.


PRs should only be marked as "deferred" if there is a clear path towards getting
the blocking issue resolved within a reasonable time frame. If a PR depends on
a more amorphous change, such as a type system change that has not yet reached
the PEP stage, it should instead be closed.

Maintainers who add the "deferred" label should state clearly what exactly the
blocker is, usually with a link to an open issue in another project.

### Closing stale PRs

To keep the number of open PRs manageable, we may close PRs when they have been
open for too long. Specifically, we close open PRs that either have failures in CI,
serious merge conflicts or unaddressed feedback, and that have not seen any
activity in three months.

We want to maintain a welcoming atmosphere for contributors, so use a friendly
message when closing the PR. Example message:

Thanks for contributing! I'm closing this PR for now, because it still
<fails some tests OR has unresolved review feedback OR has a merge conflict>
after three months of inactivity. If you are still interested, please feel free to open
a new PR (or ping us to reopen this one).