Remove unused variables #7316
Remove unused variables #7316
Conversation
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/5a7b1e44b77c039/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/5a7b1e44b77c039/output.txt Total script time: 1.11 mins Published |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/30d99a107f82b06/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/8892b91c7e5fedf/output.txt |
| @@ -295,7 +295,6 @@ var PDFViewer = (function pdfViewer() { | |||
| var onePageRendered = new Promise(function (resolve) { | |||
| resolveOnePageRendered = resolve; | |||
| }); | |||
| this.onePageRendered = onePageRendered; | |||
There was a problem hiding this comment.
I'm not sure I agree with removing this one completely, since there might still be cases where it could be useful. Note that this property is currently available in e.g. viewer components.
There was a problem hiding this comment.
Alright, let's keep this one then. I have updated the commit to put this line back. Note that the test run should still be fine because only this file changed (in /web, outside of the tests).
These have been found using `gulp lint` in combination with the `unused: true` parameter for JSHint. Unfortunately there are too many false positives to enable this feature, but now that most globals have been removed because of the conversion to UMD the results are much more useful than before.
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/30d99a107f82b06/output.txt Total script time: 21.48 mins
|
timvandermeij
force-pushed
the
remove-unused
branch
from
d23ee73 to
6a7012a
Compare
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/8892b91c7e5fedf/output.txt Total script time: 27.60 mins
|
| @@ -322,10 +322,9 @@ var PDFFindController = (function PDFFindControllerClosure() { | |||
| * @param {number} index - match index. | |||
| * @param {Array} elements - text layer div elements array. | |||
| * @param {number} beginIdx - start index of the div array for the match. | |||
| * @param {number} endIdx - end index of the div array for the match. | |||
There was a problem hiding this comment.
This was added in commit 2ac7ac4, but never appears to actually have been used, I wonder what the intention was?
There was a problem hiding this comment.
@yurydelendik Could you comment on this? Was it supposed to be used or can we remove it safely here?
There was a problem hiding this comment.
We basically want to show entire match. If scrollIntoView would accept multiple elements then showing something elements[beginIdx] through elements[endIdx] will be a better choice. Not sure what will be a right call here. I'll let you decide :)
There was a problem hiding this comment.
Since there is currently no support for that, I would say we remove the unused parameter for now. Once there is support for that, we can re-add it and actually use it in this method.
There was a problem hiding this comment.
@Snuffleupagus Do you agree with the above or do you want me to keep this variable?
There was a problem hiding this comment.
I don't have a strong opinion either way, so do what you want here :-)
|
r=me, feel free to land this once you've made a decision on #7316 (comment). |
These have been found using
gulp lintin combination with theunused: trueparameter for JSHint. Unfortunately there are too many false positives to enable this feature, but now that most globals have been removed because of the conversion to UMD the results are much more useful than before.