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

refactor(lint): use tslint api for linting #4248

Merged
merged 1 commit into from
Jan 28, 2017

Conversation

delasteve
Copy link
Contributor

Fixes #867 and #3993

@@ -41,7 +40,6 @@
"karma-remap-istanbul": "^0.2.1",
"protractor": "~4.0.13",
"ts-node": "1.2.1",
"tslint": "^4.3.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hansl please let me know if you want this line back in. It's technically not necessary anymore since it's using the CLI tslint dependency and not the project's dependency.

Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

I have three requests for this PR:

  • I don't think we should use tslint from the CLI. It removes the user's ability to update it (or codelyzer), and is generally confusing. I understand a big argument against that is that we now rely on the tslint api... but we also do that for karma. So the real answer to that is to check for compatible versions when running the command (not overly important for this PR, mostly a note for the future).
  • We need some tests for the --force and --format flags in https://github.com/angular/angular-cli/blob/master/tests/e2e/tests/test/lint.ts
  • We need to either be backwards compatible (using npm run if there's no config maybe?) or give a reasonable warning telling users what they need to change to upgrade. As is, upgraders wouldn't be able to run ng lint.

Aside from that it's great to have better linting support, and a lot of people will appreciate the --format flag as well I bet.

I should probably do the same for ng e2e.

@delasteve delasteve force-pushed the refactor/linting-ts-files branch 5 times, most recently from d246fd9 to ebf9515 Compare January 27, 2017 23:24
Closes angular#867, angular#3993

BREAKING CHANGE: In order to use the updated `ng lint` command,
  the following section will have to be added to the project's
  `angular-cli.json` at the root level of the json object.

  ```json
  "lint": [
    {
      "files": "src/**/*.ts",
      "project": "src/tsconfig.json"
    },
    {
      "files": "e2e/**/*.ts",
      "project": "e2e/tsconfig.json"
    }
  ],
  ```

Alternatively, you can run `ng update`.
@filipesilva filipesilva merged commit 0664beb into angular:master Jan 28, 2017
@filipesilva
Copy link
Contributor

Thanks for this awesome update @delasteve !

@delasteve delasteve deleted the refactor/linting-ts-files branch January 29, 2017 01:45
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
Closes angular#867, angular#3993

BREAKING CHANGE: 

In order to use the updated `ng lint` command, the following section will have to be added to the project's `angular-cli.json` at the root level of the json object.

  ```json
  "lint": [
    {
      "files": "src/**/*.ts",
      "project": "src/tsconfig.json"
    },
    {
      "files": "e2e/**/*.ts",
      "project": "e2e/tsconfig.json"
    }
  ],
  ```

Alternatively, you can run `ng update`.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
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.

All errors assumed to be lint errors
3 participants