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 so issues are not required (fixes #7015) #7072

Merged
merged 1 commit into from Sep 9, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/ISSUE_TEMPLATE.md
Expand Up @@ -8,7 +8,11 @@ This template is for bug reports. If you are reporting a bug, please continue on
Note that leaving sections blank will make it difficult for us to troubleshoot and we may have to close the issue.
-->

**What version of ESLint are you using?**
**Tell us about your environment**

* **ESLint Version:**
* **Node Version:**
* **npm Version:**
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need Node version and NPM version? We are sometimes very strict about requiring all of the fields in the template be filled out, and those two might make a difference, but only very occasionally. For most bugs they don't really matter.

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 see a downside to asking for this info. The fewer times we need to ask for more info, the faster triage goes.

Copy link
Member

Choose a reason for hiding this comment

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

The downside is higher bar of entry. If we ask for it in the template but don't insist on people providing that information if they skipped it, I'm fine with it. But requiring 100% of people to provide it when it might only be relevant in 2% of cases is an overkill.


**What parser (default, Babel-ESLint, etc.) are you using?**

Expand Down
27 changes: 20 additions & 7 deletions .github/PULL_REQUEST_TEMPLATE.md
@@ -1,16 +1,29 @@
<!--
Thanks for submitting a pull request to ESLint. Before continuing, please be sure you've read over our guidelines:
http://eslint.org/docs/developer-guide/contributing/pull-requests
**What is the purpose of this pull request? (put an "X" next to item)**

Specifically, all pull requests containing code require an **accepted** issue (documentation-only pull requests do not require an issue). If this pull request contains code and there isn't yet an issue explaining why you're submitting this pull request, please stop and open a new issue first.
```
[ ] Documentation update
[ ] Bug fix ([template](https://github.com/eslint/eslint/blob/master/templates/bug-report.md))
[ ] New rule ([template](https://github.com/eslint/eslint/blob/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://github.com/eslint/eslint/blob/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
Copy link
Member

Choose a reason for hiding this comment

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

If you want, you could also use GitHub's checkbox list feature. That would mean no code fence and each line is preceded with an unordered list token (* or -). That said, I'm okay with how this is currently done.

EDIT: You're using that further down, so I assume this was an intention choice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this was intentional. Otherwise, people can change it from the issue directly, and that's not what we want here.

```

Please answer all questions below.
-->
*If the item you've check above has a template, please paste the template questions here and answer them.*

**What issue does this pull request address?**


**Please check each item to ensure your pull request is ready:**

- [ ] I've read the [pull request guide](http://eslint.org/docs/developer-guide/contributing/pull-requests)
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to phrase this as a list of checkboxes? This feels a lot like "Yes, I've read terms and conditions" callouts. Maybe we should just mention that to make successful pull request you should do the following things?

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'd rather force them to check the boxes. People can easily skip over reading text, but actions are harder to skip.

- [ ] I've included tests for my change
- [ ] I've updated documentation for my change (if appropriate)

**What changes did you make? (Give an overview)**


**Is there anything you'd like reviewers to focus on?**


7 changes: 1 addition & 6 deletions docs/developer-guide/contributing/new-rules.md
Expand Up @@ -19,12 +19,7 @@ Even though these are the formal criteria for inclusion, each rule is evaluated

## Proposing a Rule

If you want to propose a new rule, [create an issue](https://github.com/eslint/eslint/issues/new?body=**When%20does%20this%20rule%20warn%3F%20Please%20describe%20and%20show%20example%20code%3A**%0A%0A**Is%20this%20rule%20preventing%20an%20error%20or%20is%20it%20stylistic%3F**%0A%0A**Why%20is%20this%20rule%20a%20candidate%20for%20inclusion%20instead%20of%20creating%20a%20custom%20rule%3F**%0A%0A**Are%20you%20willing%20to%20create%20the%20rule%20yourself%3F**%0A%0A) be sure to include:

1. When the rules will warn. Include a description as well as sample code.
1. Whether the rule prevents an error or is stylistic.
1. Why the rule should be in the core instead of creating a custom rule.
1. Are you willing to create the rule yourself?
If you want to propose a new rule, [create a pull request](/docs/developer-guide/contributing/pull-requests) or new issue and paste the questions from the [rule proposal template](https://github.com/eslint/eslint/blob/master/templates/rule-proposal.md) into the description.

We need all of this information in order to determine whether or not the rule is a good core rule candidate.

Expand Down
5 changes: 2 additions & 3 deletions docs/developer-guide/contributing/pull-requests.md
Expand Up @@ -8,8 +8,7 @@ If you'd like to work on a pull request and you've never submitted code before,

1. Sign our [Contributor License Agreement](https://contribute.jquery.org/cla).
1. Set up a [development environment](../development-environment).
1. Ensure there's an issue that describes what you're doing and the issue has been accepted. You can create a new issue or just indicate you're [working on an existing issue](working-on-issues).
* Exception: documentation-only changes do not require an issue.
1. If you want to implement a breaking change or a change to the core, ensure there's an issue that describes what you're doing and the issue has been accepted. You can create a new issue or just indicate you're [working on an existing issue](working-on-issues). Bug fixes, documentation changes, and other pull requests do not require an issue.

After that, you're ready to start working on code.

Expand Down Expand Up @@ -71,7 +70,7 @@ The `Tag` is one of the following:

Use the [labels of the issue you are working on](working-on-issues#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. The issue number should be mentioned at the end. If the commit doesn't completely fix the issue, then use `(refs #1234)` instead of `(fixes #1234)`.
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 at the end. 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:

Expand Down
13 changes: 1 addition & 12 deletions docs/developer-guide/contributing/reporting-bugs.md
@@ -1,17 +1,6 @@
# Reporting Bugs

If you think you've found a bug in ESLint, please [create a new issue](https://github.com/eslint/eslint/issues/new?body=**What%20version%20are%20you%20using%3F**%0A%0A**What%20did%20you%20do%3F**%0A%0A**What%20happened%3F**%0A%0A**What%20did%20you%20expect%20to%20happen%3F**%0A%0A) on GitHub. Be sure to include the following information:

1. The version of ESLint you are using.
1. What you did.
1. What you expected to happen.
1. What actually happened.

In addition, if you're reporting a bug with a rule, be sure to include:

* The actual source code that is causing the issue
* Your configuration
* The complete and actual output from ESLint (don't summarize, we need the raw output)
If you think you've found a bug in ESLint, please [create a new issue](https://github.com/eslint/eslint/issues/new) or a [pull request](/docs/developer-guide/contributing/pull-requests) on GitHub. Be sure to copy the questions from the [bug report template](https://github.com/eslint/eslint/blob/master/templates/bug-report.md).

Please include as much detail as possible to help us properly address your issue. If we need to triage issues and constantly ask people for more detail, that's time taken away from actually fixing issues. Help us be as efficient as possible by including a lot of detail in your issues.

Expand Down
7 changes: 1 addition & 6 deletions docs/developer-guide/contributing/rule-changes.md
Expand Up @@ -4,12 +4,7 @@ Occasionally, a core ESLint rule needs to be changed. This is not necessarily a

## Proposing a Rule Change

If you want to propose a rule change, [create an issue](https://github.com/eslint/eslint/issues/new?body=**What%20version%20of%20ESLint%20are%20you%20using%3F**%0A%0A**What%20rule%20do%20you%20want%20to%20change%3F**%0A%0A**What%20code%20should%20be%20flagged%20as%20incorrect%20with%20this%20change%3F**%0A%0A**What%20happens%20when%20the%20rule%20is%20applied%20to%20this%20code%20now%3F**%0A%0A%0A) and be sure to include:

1. The version of ESLint you are using
2. The rule you want to change
3. The code you want to be flagged as incorrect
4. What happens when the rule is applied to the code without your change
To propose a change to an existing rule, [create a new issue](https://github.com/eslint/eslint/issues/new) or a [pull request](/docs/developer-guide/contributing/pull-requests) on GitHub. Be sure to copy the questions from the [rule change proposal template](https://github.com/eslint/eslint/blob/master/templates/rule-change-proposal.md).

We need all of this information in order to determine whether or not the change is a good candidate for inclusion.

Expand Down
1 change: 0 additions & 1 deletion docs/maintainer-guide/pullrequests.md
Expand Up @@ -12,7 +12,6 @@ When a pull request is opened, the bot will check the following:

1. Has the submitter signed a CLA?
1. Is the commit message summary in the correct format? Double-check that the tag ("Fix:", "New:", etc.) is correct based on the issue. Documentation-only pull requests do not require an issue.
1. Is there only one commit in the pull request?
1. Does the commit summary reference an issue?
1. Is the commit summary too long?

Expand Down
16 changes: 16 additions & 0 deletions templates/bug-report.md
@@ -0,0 +1,16 @@
**Tell us about your environment**

* **ESLint Version:**
* **Node Version:**
Copy link
Member

Choose a reason for hiding this comment

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

Same as issue template.

* **npm Version:**

**What parser (default, Babel-ESLint, etc.) are you using?**

**Please show your full configuration:**

**What did you do? Please include the actual source code causing the issue.**

**What did you expect to happen?**

**What actually happened? Please include the actual, raw output from ESLint.**

9 changes: 0 additions & 9 deletions templates/pr-create.md.ejs
Expand Up @@ -8,11 +8,6 @@ function isValidCommitFlag(log) {
return !!result || log.indexOf("Revert \"") === 0;
}

function needsIssueRef(log) {
var result = log.match(ISSUE_REF_PATTERN);
return !result && log.indexOf("Docs:") !== 0;
}

var problems = [];

// Check for one commit per pull request
Expand All @@ -27,10 +22,6 @@ if (meta.commits) {
if (log.length > 72) {
problems.push("The commit summary must be 72 characters or shorter. Please check out our [guide](http://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes) for how to properly format your commit summary and [update](http://eslint.org/docs/developer-guide/contributing/pull-requests#updating-the-commit-message) it on this pull request.");
}

if (needsIssueRef(log)) {
problems.push("Pull requests with code require an issue to be mentioned at the end of the commit summary, such as `(fixes #1234)`. Please [update](http://eslint.org/docs/developer-guide/contributing/pull-requests#updating-the-commit-message) the commit summary with an issue (file a new issue if one doesn't already exist).")
}
}

if (problems.length) { %>
Expand Down
19 changes: 19 additions & 0 deletions templates/rule-change-proposal.md
@@ -0,0 +1,19 @@
**What rule do you want to change?**


**Does this change cause the rule to produce more or fewer warnings?**


**How will the change be implemented? (New option, new default behavior, etc.)?**


**Please provide some example code that this change will affect:**

```js
<!-- example code here -->
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a js comment syntax instead, since it's inside a "```js" code block?

/* example code here */

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The html comment won't be output of its left in by mistake, a JS comment will

```

**What does the rule currently do for this code?**


**What will the rule do after it's changed?**
19 changes: 19 additions & 0 deletions templates/rule-proposal.md
@@ -0,0 +1,19 @@
**Please describe what the rule should do:**


**What category of rule is this? (place an "X" next to just one item)**

```
[ ] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)
```

**Provide 2-3 code examples that this rule will warn about:**

```js
<!-- put your code examples here -->
Copy link
Member

Choose a reason for hiding this comment

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

Same for this one here!

```

**Why should this rule be included in ESLint (instead of a plugin)?**
28 changes: 0 additions & 28 deletions tests/templates/pr-create.md.ejs.js
Expand Up @@ -61,7 +61,6 @@ describe("pr-create.md.ejs", function() {
});

assert.ok(result.indexOf("begin with a tag") > -1);
assert.ok(result.indexOf("require an issue") > -1);
});

it("should mention commit message length when there's a message longer than 72 characters", function() {
Expand All @@ -85,7 +84,6 @@ describe("pr-create.md.ejs", function() {
});

assert.ok(result.indexOf("72 characters") > -1);
assert.ok(result.indexOf("require an issue") === -1);
});

it("should not mention commit message length when there's a multi-line message with first line not over 72 characters", function() {
Expand All @@ -109,7 +107,6 @@ describe("pr-create.md.ejs", function() {
});

assert.equal(result.trim(), "LGTM");
assert.ok(result.indexOf("require an issue") === -1);
});

it("should not mention missing issue when there's one documentation commit", function() {
Expand All @@ -133,7 +130,6 @@ describe("pr-create.md.ejs", function() {
});

assert.equal(result.trim(), "LGTM");
assert.ok(result.indexOf("require an issue") === -1);
});

["Breaking", "Build", "Chore", "Docs", "Fix", "New", "Update", "Upgrade"].forEach(function(type) {
Expand All @@ -158,31 +154,7 @@ describe("pr-create.md.ejs", function() {
});

assert.equal(result.trim(), "LGTM");
assert.ok(result.indexOf("require an issue") === -1);
});
});

it("should mention missing issue when there's a missing closing paren", function() {
const result = ejs.render(TEMPLATE_TEXT, {
payload: {
sender: {
login: "nzakas"
},
commits: 1
},
meta: {
cla: true,
commits: [
{
commit: {
message: "Fix: Foo bar (fixes #1234"
}
}
]
}
});

assert.ok(result.indexOf("require an issue") > -1);
});

});