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

build: no longer run tslint from within gulp task #35800

Closed

Conversation

devversion
Copy link
Member

@devversion devversion commented Mar 2, 2020

Switches our tslint setup to the standard tslint.json linter excludes.
The set of files that need to be linted is specified through a Yarn script.

For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.

We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.

This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.

Additionally, linting of d.ts files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.

Resolves DEV-47

The old version we currently use does not properly implement the
option to exclude files from within the `tslint.json` file.
Switches our tslint setup to the standard `tslint.json` linter excludes.
The set of files that need to be linted is specified through a Yarn script.

For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.

We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.

This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.

Additionally, linting of `d.ts` files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.
@devversion devversion force-pushed the build/switch-away-gulp-tslint branch from 5f38c95 to 0a875b7 Compare March 2, 2020 18:13
@devversion devversion marked this pull request as ready for review March 2, 2020 18:34
@devversion devversion added area: build & ci Related the build and CI infrastructure of the project action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Mar 2, 2020
@ngbot ngbot bot modified the milestone: needsTriage Mar 2, 2020
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 3, 2020
atscott pushed a commit that referenced this pull request Mar 3, 2020
The old version we currently use does not properly implement the
option to exclude files from within the `tslint.json` file.

PR Close #35800
atscott pushed a commit that referenced this pull request Mar 3, 2020
Switches our tslint setup to the standard `tslint.json` linter excludes.
The set of files that need to be linted is specified through a Yarn script.

For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.

We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.

This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.

Additionally, linting of `d.ts` files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.

PR Close #35800
@atscott atscott closed this in 5349e46 Mar 3, 2020
atscott pushed a commit that referenced this pull request Mar 3, 2020
Switches our tslint setup to the standard `tslint.json` linter excludes.
The set of files that need to be linted is specified through a Yarn script.

For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.

We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.

This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.

Additionally, linting of `d.ts` files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.

PR Close #35800
@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 Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants