Skip to content

docs: mention testing changes and keeping PRs up-to-date in PR docs #9246

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

Merged
Show file tree
Hide file tree
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
17 changes: 0 additions & 17 deletions docs/contributing/Local_Development.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,6 @@ You can run `yarn test` in any package to run its tests.

> [VS Code launch tasks](https://code.visualstudio.com/docs/editor/tasks) tasks are provided that allow [visual debugging](https://code.visualstudio.com/docs/editor/debugging) tests.

#### Code Coverage

We aim for 100% code coverage in all PRs when possible, except in the `website/` package.
Coverage reports are generated locally whenever `yarn test` is run.

The `codecov` bot should also comment on your PR with the percentage, as well as links to the line-by-line coverage of each file touched by your PR.

#### Granular Unit Tests

Most tests in most packages are set up as small, self-contained unit tests.
We generally prefer that to keep tests and their failure reports granular.

For rule tests we recommend, when reasonable:

- Including both `valid` and `invalid` code changes in most PRs that affect rule behavior
- Limiting to one error per `invalid` case

### Type Checking
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. should this section move too? 🤔

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 don't think so - I see this page as the "here are the raw commands you'll use".


All code should pass TypeScript type checking.
Expand Down
29 changes: 28 additions & 1 deletion docs/contributing/Pull_Requests.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,26 @@ Please don't:
- Comment on a closed PR
- Reasoning: It is much easier for maintainers to not lose track of things if they are posted as issues. If you think there's a bug in typescript-eslint, the right way to ask is to [file a new issue](https://github.com/typescript-eslint/typescript-eslint/issues/new/choose). The issue templates include helpful & necessary practices such as making sure you're on the latest version of all our packages. You can provide the link to the relevant PR to add more context.

### Raising a PR
### Testing Changes

#### Code Coverage

We aim for 100% code coverage in all PRs when possible, except in the `website/` package.
Coverage reports are generated locally whenever `yarn test` is run.

The `codecov` bot also comments on each PR with its percentage, as well as links to the line-by-line coverage of each file touched by the PR.

#### Granular Unit Tests

Most tests in most packages are set up as small, self-contained unit tests.
We generally prefer that to keep tests and their failure reports granular.

For rule tests we recommend, when reasonable:

- Including both `valid` and `invalid` code changes in most PRs that affect rule behavior
- Limiting to one error per `invalid` case

### Raising the PR

Once your changes are ready, you can raise a PR! 🙌
The title of your PR should match the following format:
Expand Down Expand Up @@ -84,6 +103,14 @@ Once you've addressed all our feedback by making code changes and/or started a f

Once the feedback is addressed, and the PR is approved, we'll ensure the branch is up to date with `main`, and merge it for you.

#### Updating Over Time

You generally don't need to keep merging commits from `main` into your PR branch.
If you see merge conflicts or other intersections that worry you, then you can preemptively - but that's optional.

If we think merge conflicts need to be resolved in order to merge and/or review a PR, we might do that for you as a courtesy _if_ we have time.
If not, we may ask you to.

### Asking Questions

If you need help and/or have a question, posting a comment in the PR is a great way to do so.
Expand Down
Loading