Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add "tslint:all" configuration, and use it to lint tslint itself #2241

Closed
wants to merge 3 commits into from

Conversation

andy-hanson
Copy link
Contributor

PR checklist

Overview of change:

Created tslint:all by using all rules listed at https://palantir.github.io/tslint/rules/ on the strictest settings.
Contributors will have to update it manually when they add a new rule.

CHANGELOG.md entry:

[api] tslint:all configuration

@andy-hanson
Copy link
Contributor Author

Test failures fixed by #2239.

@andy-hanson
Copy link
Contributor Author

Further test failures fixed by #2234.

@nchen63
Copy link
Contributor

nchen63 commented Mar 6, 2017

should probably write a test to verify that all rules are in all instead of relying on PR checklist

@andy-hanson
Copy link
Contributor Author

OK. There was a describe.only from #2284 so a few tests were failing. Updated some and made #2329 for another.

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

sorry, this is too much to merge as a single PR. there should be separate PRs to enable tests, add this configuration, and change the overall project's linting config.

],
};

// Exclude typescript-only rules from jsRules
Copy link
Contributor

Choose a reason for hiding this comment

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

we should be able to infer this from rule metadata. if the metadata is wrong, let's fix that

@@ -26,7 +26,7 @@ import {
import { IOptions } from "./../src/language/rule/rule";
import { createTempFile } from "./utils";

describe.only("Configuration", () => {
describe("Configuration", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

awkward. we should ban describe.only and it.only in this project's tslint config

@@ -47,7 +48,9 @@ describe("TAP Formatter", () => {
getFailureString(3, "last-name", "error", TEST_FILE, 0, 12, "last failure");

const actualResult = formatter.format(failures);
assert.equal(actualResult, `TAP version 13\n1..${failures.length}\n` + expectedResult);
if (!true) { // TODO: #2329
Copy link
Contributor

Choose a reason for hiding this comment

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

please use it.skip instead of this syntax

let testFailed = false;
for (const { added, removed, value } of diffResults) {
if (added) {
console.warn(`Rule in 'tslint:all' does not exist: ${value}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of warning, let's report this message in the assertion on L153

"prefer-const": true,
"newline-before-return": false,
"no-any": false,
"no-inferrable-types": false, // TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

what is TODO supposed to indicate here?

@andy-hanson
Copy link
Contributor Author

Started with #2406.

@andy-hanson andy-hanson mentioned this pull request Mar 28, 2017
4 tasks
@adidahiya
Copy link
Contributor

closing in favor of #2406 and #2417

@adidahiya adidahiya closed this Mar 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants