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

docs: update "Submit a Pull Request" page #17712

Merged
merged 5 commits into from Nov 13, 2023
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
101 changes: 45 additions & 56 deletions docs/src/contribute/pull-requests.md
Expand Up @@ -53,45 +53,9 @@ git add -A
git commit
```

All ESLint projects follow [Conventional Commits](https://www.conventionalcommits.org/) for our commit messages. (Note: we don’t support the optional scope in messages.) Here's an example commit message:
The commit messages do not need to be in any specific format because they do not show up in the changelog.
Copy link
Member

Choose a reason for hiding this comment

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

I still think we should be encouraging people to format their commit messages in Conventional Commits format. I'd rather not need to manually edit pull request titles all the time.

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 see. Then it would be good to keep information about the commit message here and describe what is specific to the ESLint PR process. I'll change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted the part about Conventional Commits, should be okay now.


```txt
tag: Short description of what you did

Longer description here if necessary

Fixes #1234
```

The first line of the commit message (the summary) must have a specific format. This format is checked by our build tools.

The `tag` is one of the following:

* `fix` - for a bug fix.
* `feat` - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
* `fix!` - for a backwards-incompatible bug fix.
* `feat!` - for a backwards-incompatible enhancement or feature.
* `docs` - changes to documentation only.
* `chore` - for changes that aren't user-facing.
* `build` - changes to build process only.
* `refactor` - a change that doesn't affect APIs or user experience.
* `test` - just changes to test files.
* `ci` - changes to our CI configuration files and scripts.
* `perf` - a code change that improves performance.

Use the [labels of the issue you are working on](work-on-issue#issue-labels) to determine the best tag.

The message summary should be a one-sentence description of the change, and it must be 72 characters in length or shorter. If the pull request addresses an issue, then the issue number should be mentioned in the body of the commit message in the format `Fixes #1234`. If the commit doesn't completely fix the issue, then use `Refs #1234` instead of `Fixes #1234`.

Here are some good commit message summary examples:

```txt
build: Update Travis to only test Node 0.10
fix: Semi rule incorrectly flagging extra semicolon
chore: Upgrade Esprima to 1.2, switch to using comment attachment
```

The commit message format is important because these messages are used to create a changelog for each release. The tag and issue number help to create more consistent and useful changelogs.
Note though that when you create a pull request in the GitHub UI, the title field will be prefilled with the summary (the first line in the message) of the first commit. This can be edited as required.

mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
### Step 3: Rebase onto upstream<a name="step3"></a>

Expand All @@ -116,9 +80,6 @@ If there are any failing tests, update your code until all tests pass.

With your code ready to go, this is a good time to double-check your submission to make sure it follows our conventions. Here are the things to check:

* Make sure your commit is formatted correctly.
* The pull request must have a description. The description should explain what you did and how its effects can be seen.
Copy link
Member Author

Choose a reason for hiding this comment

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

A pull request hasn't been created yet, so I guess this fits better in Step 7 down below.

* The commit message is properly formatted.
* The change introduces no functional regression. Be sure to run `npm test` to verify your changes before submitting a pull request.
* Make separate pull requests for unrelated changes. Large pull requests with multiple unrelated changes may be closed without merging.
* All changes must be accompanied by tests, even if the feature you're working on previously had no tests.
Expand All @@ -145,28 +106,58 @@ Now you're ready to send the pull request. Go to your ESLint fork and then follo

In order to submit code or documentation to an ESLint project, you’ll be asked to sign our CLA when you send your first pull request. (Read more about the Open JS Foundation CLA process at <https://cla.openjsf.org/>.)

## Following Up
The pull request must have a title and a description. The title must be properly formatted and the description should explain what you did and how its effects can be seen.

Once your pull request is sent, it's time for the team to review it. As such, please make sure to:
All ESLint projects follow [Conventional Commits](https://www.conventionalcommits.org/) for our pull request titles. (Note: we don’t support the optional scope in messages.) Here's an example pull request title:

1. Monitor the status of the Travis CI build for your pull request. If it fails, please investigate why. We cannot merge pull requests that fail Travis for any reason.
1. Respond to comments left on the pull request from team members. Remember, we want to help you land your code, so please be receptive to our feedback.
1. We may ask you to make changes, rebase, or squash your commits.
```txt
tag: Short description of what you did
```

### Updating the Commit Message
The title of the pull request must have a specific format. This format is checked by our build tools.

If your commit message is in the incorrect format, you'll be asked to update it. You can do so via:
The `tag` is one of the following:

```shell
git commit --amend
```
* `fix` - for a bug fix.
* `feat` - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
* `fix!` - for a backwards-incompatible bug fix.
* `feat!` - for a backwards-incompatible enhancement or feature.
* `docs` - changes to documentation only.
* `chore` - for changes that aren't user-facing.
* `build` - changes to build process only.
* `refactor` - a change that doesn't affect APIs or user experience.
* `test` - just changes to test files.
* `ci` - changes to our CI configuration files and scripts.
* `perf` - a code change that improves performance.

This will open up your editor so you can make changes. After that, you'll need to do a forced push to your branch:
Use the [labels of the issue you are working on](work-on-issue#issue-labels) to determine the best tag.

```shell
git push origin issue1234 -f
The PR title should be a one-sentence description of the change, and it must be 72 characters in length or shorter. If the pull request addresses an issue, then the issue number should be mentioned in the PR description in the format `Fixes #1234`. If the pull request doesn't completely fix the issue, then use `Refs #1234` instead of `Fixes #1234`.

Here are some good pull request title examples:

```txt
build: Update Travis to only test Node 0.10
fix: Semi rule incorrectly flagging extra semicolon
chore: Upgrade Esprima to 1.2, switch to using comment attachment
```

When a pull request is merged, its commits will be squashed into one single commit. The first line of the squashed commit message will contain the title of the pull request.

The pull request title format is important because the titles are used to create a changelog for each release. The tag and issue number help to create more consistent and useful changelogs.

## Following Up

Once your pull request is sent, it's time for the team to review it. As such, please make sure to:

1. Monitor the status of the GitHub Actions CI build for your pull request. If it fails, please investigate why. We cannot merge pull requests that fail the CI build for any reason.
1. Respond to comments left on the pull request from team members. Remember, we want to help you land your code, so please be receptive to our feedback.
1. We may ask you to make changes, rebase, or squash your commits.

### Updating the Pull Request Title

If your pull request title is in the incorrect format, you'll be asked to update it. You can do so via the GitHub user interface.

### Updating the Code

If we ask you to make code changes, there's no need to close the pull request and create a new one. Just go back to the branch on your fork and make your changes. Then, when you're ready, you can add your changes into the branch:
Expand All @@ -179,8 +170,6 @@ git push origin issue1234

When updating the code, it's usually better to add additional commits to your branch rather than amending the original commit, because reviewers can easily tell which changes were made in response to a particular review. When we merge pull requests, we will squash all the commits from your branch into a single commit on the `main` branch.

The commit messages in subsequent commits do not need to be in any specific format because these commits do not show up in the changelog.

### Rebasing

If your code is out-of-date, we might ask you to rebase. That means we want you to apply your changes on top of the latest upstream code. Make sure you have set up a [development environment](./development-environment) and then you can rebase using these commands:
Expand Down