poorejc commented on pull request #234:
URL:
https://github.com/apache/incubator-flagon-useralejs/pull/234#issuecomment-1079833413
Lots more debugging tonight:
- this code seems to initiate setup() before new configs are merged. Because
default for autostart is 'true'; it always initiates setup().
```
if (config.autostart) {
setup(config);
}
```
- @kevbrowndev code adds another logic check inside setup(); even though
setup() has been initiated by the above code, it ends up timing out because it
fails the logic check after page is interactive when autostart is 'false'.
- The additional logic check for autostart within setup(), along with adding
config.autostart === false to the stop() function fixes stop().
Punchline: I think the PR adds back expected functionality, which was
actually missing. When autostart = false, logging doesn't start & start() works
appropriately. When autostart = true, logging starts and stop() works. I think
that's an enhancement. I'll give a few more tests and a some time for
@UncleGedd to weigh in, but then we should pull this PR into test and then
master branch.
@kevbrowndev commits and my working mods and testing well on this
branch[https://github.com/apache/incubator-flagon-useralejs/tree/setupStartStopRefactor]
Following that we should think about refactoring how we initiate setup()
(silly to initiate off (broken) autostart logic check before newConfigs are
merged...), and how to re-work the order of operations for UserALE.js startup
so that newConfigs can update config before setup() executes (maybe run configs
off a readystate trigger with timeout function...).
--
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: dev-unsubscr...@flagon.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
