style(*): remove unused variables #13701
Conversation
| @@ -1,6 +1,6 @@ | |||
| 'use strict'; | |||
|
|
|||
| var $compileMinErr = minErr('$compile'); | |||
| var $templateRequestMinErr = minErr('$compile'); | |||
There was a problem hiding this comment.
You could also just reuse the already created $compileMinErr.
There was a problem hiding this comment.
I figured this file shouldn't depend on compile.js like that. Might want to also remove the '$compile' string but I figured that would be another change...
|
LGTM |
| } else { | ||
| parsedNgModelAssign($scope, ctrl.$modelValue); | ||
| parsedNgModelAssign($scope, newValue); |
There was a problem hiding this comment.
@lgalfaso are these two changes correct?
There was a problem hiding this comment.
this is fine as ngModelSet is only called from
https://github.com/jbedard/angular.js/blob/unused/src/ng/directive/ngModel.js#L755
|
In that case LGTM |
|
jshint has an unused rule. Let's see what happens when we use that. |
|
Currently that doesn't work very well because of how variables are shared across files and jshint is run on the src files. If jshint was instead run on the resulting angular.js it would work, or maybe if shared variables are marked as globals or the rule is disabled for those vars? |
|
Many files use jshint global already. Some variables actually show up as undefined if they are not marked global. But maybe that's just my personal setup. |
|
Ah, I confused unused with undef ... So yes, directives, providers, services all trigger unused because they are only used in AngularPublic.js. This file already has a global definition, it's just that a lot of functions and variables are missing. |
... and in some cases make use of unused vars, and rename a duplicate var.
It would be nice of some of these could be caught by the build...