You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/05/10 16:45:08 UTC
[GitHub] [accumulo] tchaie opened a new pull request, #2687: Add declaration for scansList prior to usage
tchaie opened a new pull request, #2687:
URL: https://github.com/apache/accumulo/pull/2687
On the `Active Scans` page in the Monitor, the console complains about `scansList` not being defined.
This update was made using `compactions.js` as a reference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii merged pull request #2687: Add declaration for scansList prior to usage
Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #2687:
URL: https://github.com/apache/accumulo/pull/2687
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] tchaie commented on a diff in pull request #2687: Add declaration for scansList prior to usage
Posted by GitBox <gi...@apache.org>.
tchaie commented on code in PR #2687:
URL: https://github.com/apache/accumulo/pull/2687#discussion_r869669091
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js:
##########
@@ -18,6 +18,8 @@
*/
"use strict";
+var scansList;
Review Comment:
`if (scansList)
scansList.ajax.reload(null, false);`
In the other method, an if statement checks for the existence of scansList, so I think it would be referencing the same scansList var created in the previous method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2687: Add declaration for scansList prior to usage
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2687:
URL: https://github.com/apache/accumulo/pull/2687#discussion_r869640962
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js:
##########
@@ -18,6 +18,8 @@
*/
"use strict";
+var scansList;
Review Comment:
Are they referencing the same value, or just reusing a variable name? If they are just reusing a variable name, both can be locally scoped. If the idea is to declare a global in one method, and reading the variable in the other, then this change is probably what we want. I'm not sure which is the case without looking at the other method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2687: Add declaration for scansList prior to usage
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2687:
URL: https://github.com/apache/accumulo/pull/2687#discussion_r869605222
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js:
##########
@@ -18,6 +18,8 @@
*/
"use strict";
+var scansList;
Review Comment:
Can this be local to the function?
As in `var scansList = $('#scansList'). ...`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] tchaie commented on a diff in pull request #2687: Add declaration for scansList prior to usage
Posted by GitBox <gi...@apache.org>.
tchaie commented on code in PR #2687:
URL: https://github.com/apache/accumulo/pull/2687#discussion_r869638236
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js:
##########
@@ -18,6 +18,8 @@
*/
"use strict";
+var scansList;
Review Comment:
`scansList` is also used in the `refreshScansTable` function. Wouldn't it complain from being out of scope?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org