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.

Sign up for GitHub

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

Jump to bottom

build: replace deprecated tslint rule with compiler option #13186

Merged

vivian-hu-zz merged 1 commit into angular:master from crisbeto:tslint-no-unused-variable

Dec 12, 2018

Member

crisbeto commented Sep 18, 2018

Removes the no-unused-variable tslint rule which has been logging out a deprecation warning, with the equivalent compiler option.

crisbeto added the in progress label

crisbeto self-assigned this

crisbeto requested review from amcdnl, devversion, jelbourn, josephperrott and mmalerba as code owners

September 18, 2018 19:55

googlebot added the cla: yes label

devversion approved these changes

View reviewed changes

Member

devversion left a comment •

edited

Loading

I've wanted to do that for a long time, but didn't bother fixing because I assumed that live-reload won't work properly.

Did you notice something similar as well? This was the reason why TSLint re-introduced the rule at some point.

devversion added the pr: lgtm label

Member Author

crisbeto commented Sep 18, 2018

Livereload should work fine since now it'll be treated as just another compiler error. That being said, I would've preferred it to stay a lint check, because having compiler errors for unused variables can be annoying when you're debugging something. Thankfully vscode recently started highlighting them.

Member

devversion commented Sep 18, 2018 •

edited

Loading

That's reasonable. At some point I remember the whole livereload process being killed if an error occurred at compilation, but that shouldn't be much different to the noUnusedParameters option which has been already enabled at some point.

Member

jelbourn commented Sep 18, 2018

I would also prefer keeping this as a lint check, MAYBE even go so far as to copy the rule once tslint deletes it. I feel pretty strongly that a compiler has no business enforcing something like no-unused-params, especially while actively doing development.

Member Author

crisbeto commented Sep 18, 2018

It looks like a fair bit of logic that we'd have to fork and maintain though. Also judging by the discussion on palantir/tslint#3919, it may not be trivial to continue supporting it ourselves.

Member

jelbourn commented Sep 18, 2018

LGTM, I guess

jelbourn added action: merge and removed in progress labels

ngbot bot commented Sep 18, 2018

I see that you just added the pr: merge ready label, but the following checks are still failing:
missing required label: "target: *"
status "continuous-integration/travis-ci/pr" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

jelbourn added the target: major label

crisbeto removed the action: merge label

Member Author

crisbeto commented Sep 18, 2018

Removing the merge: ready for now. For some reason the e2e tests started failing, even though their build succeeds. I'll re-apply it when it's sorted out.

crisbeto force-pushed the tslint-no-unused-variable branch from 2d1c45b to e18b12f Compare

September 19, 2018 18:23

crisbeto added the action: merge label

crisbeto removed their assignment

josephperrott approved these changes

View reviewed changes

ngbot bot commented Oct 15, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

4 similar comments

ngbot bot commented Oct 15, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

ngbot bot commented Oct 15, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

ngbot bot commented Oct 15, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

ngbot bot commented Oct 15, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

crisbeto force-pushed the tslint-no-unused-variable branch from e18b12f to b0ee89a Compare

October 15, 2018 16:50

crisbeto added target: patch and removed target: major labels

crisbeto force-pushed the tslint-no-unused-variable branch from b0ee89a to c875f56 Compare

October 28, 2018 18:05


          build: replace deprecated tslint rule with compiler option

f5c5230

Removes the `no-unused-variable` tslint rule which has been logging out a deprecation warning, with the equivalent compiler option.

crisbeto force-pushed the tslint-no-unused-variable branch from c875f56 to f5c5230 Compare

November 18, 2018 17:24

vivian-hu-zz merged commit 5dfa45f into angular:master

vivian-hu-zz added a commit that referenced this pull request


          Revert "build: replace deprecated tslint rule with compiler option (#…

0832de5

…13186)"

This reverts commit 5dfa45f.

vivian-hu-zz added a commit that referenced this pull request


          Revert "fix(snack-bar): announcing same message twice to screen reade…

e9466a4

…rs" (#14494)

* Revert "build: update nodejs bazel rules (#14466)"

This reverts commit a3fef46.

* Revert "docs(form-field): native select reset values not working in examples (#14236)"

This reverts commit 97f52c0.

* Revert "fix(menu): allow text wrapping in menu items (#11430)"

This reverts commit 3661abe.

* Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes (#13065)"

This reverts commit fe96f38.

* Revert "build: replace deprecated tslint rule with compiler option (#13186)"

This reverts commit 5dfa45f.

* Revert "fix(snack-bar): announcing same message twice to screen readers (#13298)"

This reverts commit 3fb4b23.

vivian-hu-zz added a commit that referenced this pull request


          Revert "build: replace deprecated tslint rule with compiler option (#…

80c299f

…13186)"

This reverts commit 5dfa45f.

vivian-hu-zz added a commit that referenced this pull request


          Revert "fix(autocomplete): auto-highlighted first option not display …

4d42a90

…correctly if the floating label is disabled" (#14505)

* Revert "Revert "fix(snack-bar): announcing same message twice to screen readers" (#14494)"

This reverts commit e9466a4.

* Revert "Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes" (#14492)"

This reverts commit 3eb7e9c.

* Revert "Revert "fix(menu): allow text wrapping in menu items" (#14493)"

This reverts commit a3da8d2.

* Revert "build: update nodejs bazel rules (#14466)"

This reverts commit a3fef46.

* Revert "docs(form-field): native select reset values not working in examples (#14236)"

This reverts commit 97f52c0.

* Revert "fix(menu): allow text wrapping in menu items (#11430)"

This reverts commit 3661abe.

* Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes (#13065)"

This reverts commit fe96f38.

* Revert "build: replace deprecated tslint rule with compiler option (#13186)"

This reverts commit 5dfa45f.

* Revert "fix(snack-bar): announcing same message twice to screen readers (#13298)"

This reverts commit 3fb4b23.

* Revert "feat(drag-drop): allow entire group of drop lists to be disabled (#14179)"

This reverts commit 94e76de.

* Revert "fix(drag-drop): prevent text selection while dragging on Safari (#14405)"

This reverts commit 220e388.

* Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled (#14414)"

This reverts commit 53d2b58.

josephperrott pushed a commit to josephperrott/components that referenced this pull request


          build: replace deprecated tslint rule with compiler option (angular#1…

6bbcf15

…3186)

Removes the `no-unused-variable` tslint rule which has been logging out a deprecation warning, with the equivalent compiler option.

josephperrott pushed a commit to josephperrott/components that referenced this pull request


          Revert "fix(snack-bar): announcing same message twice to screen reade…

ec2d2de

…rs" (angular#14494)

* Revert "build: update nodejs bazel rules (angular#14466)"

This reverts commit a3fef46.

* Revert "docs(form-field): native select reset values not working in examples (angular#14236)"

This reverts commit 97f52c0.

* Revert "fix(menu): allow text wrapping in menu items (angular#11430)"

This reverts commit 3661abe.

* Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes (angular#13065)"

This reverts commit fe96f38.

* Revert "build: replace deprecated tslint rule with compiler option (angular#13186)"

This reverts commit 5dfa45f.

* Revert "fix(snack-bar): announcing same message twice to screen readers (angular#13298)"

This reverts commit 3fb4b23.

josephperrott pushed a commit to josephperrott/components that referenced this pull request


          Revert "fix(autocomplete): auto-highlighted first option not display …

a7074af

…correctly if the floating label is disabled" (angular#14505)

* Revert "Revert "fix(snack-bar): announcing same message twice to screen readers" (angular#14494)"

This reverts commit e9466a4.

* Revert "Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes" (angular#14492)"

This reverts commit 3eb7e9c.

* Revert "Revert "fix(menu): allow text wrapping in menu items" (angular#14493)"

This reverts commit a3da8d2.

* Revert "build: update nodejs bazel rules (angular#14466)"

This reverts commit a3fef46.

* Revert "docs(form-field): native select reset values not working in examples (angular#14236)"

This reverts commit 97f52c0.

* Revert "fix(menu): allow text wrapping in menu items (angular#11430)"

This reverts commit 3661abe.

* Revert "fix(dialog,bottom-sheet): invert backdrop color on dark themes (angular#13065)"

This reverts commit fe96f38.

* Revert "build: replace deprecated tslint rule with compiler option (angular#13186)"

This reverts commit 5dfa45f.

* Revert "fix(snack-bar): announcing same message twice to screen readers (angular#13298)"

This reverts commit 3fb4b23.

* Revert "feat(drag-drop): allow entire group of drop lists to be disabled (angular#14179)"

This reverts commit 94e76de.

* Revert "fix(drag-drop): prevent text selection while dragging on Safari (angular#14405)"

This reverts commit 220e388.

* Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled (angular#14414)"

This reverts commit 53d2b58.

angular-automatic-lock-bot bot commented Sep 10, 2019

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 bot locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

devversion devversion approved these changes

josephperrott josephperrott approved these changes

amcdnl Awaiting requested review from amcdnl

jelbourn Awaiting requested review from jelbourn

mmalerba Awaiting requested review from mmalerba

Labels

action: merge cla: yes target: patch