-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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.
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
Remove the internal PDFScriptingManager._pageEventsReady
boolean (PR 13074 follow-up)
#13577
Remove the internal PDFScriptingManager._pageEventsReady
boolean (PR 13074 follow-up)
#13577
Conversation
…R 13074 follow-up) With the introduction of `PDFScriptingManager._closeCapability` in PR 13074, the pre-existing `PDFScriptingManager._pageEventsReady` boolean essentially became redundant. Given that you always want to avoid tracking closely related state *separately*, since it's easy to introduce subtle bugs that way, we should just remove `PDFScriptingManager._pageEventsReady` now. Obviously I *should* have done this already back in PR 13074, sorry about the churn here!
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/2c3d9350d7506cf/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 1 Live output at: http://3.101.106.178:8877/2c21d50a95ddbd7/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/2c3d9350d7506cf/output.txt Total script time: 4.37 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/2c21d50a95ddbd7/output.txt Total script time: 5.56 mins
|
…itializing the scripting-factory This way, we'll immediately clean-up in exactly the same way as the other failure code-paths in the `PDFScriptingManager.setDocument` method.
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/c065980ea7f783a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/bc3368cc2c186d3/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/c065980ea7f783a/output.txt Total script time: 4.38 mins
|
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/bc3368cc2c186d3/output.txt Total script time: 5.34 mins
|
Thanks! |
With the introduction of
PDFScriptingManager._closeCapability
in PR #13074, the pre-existingPDFScriptingManager._pageEventsReady
boolean essentially became redundant.Given that you always want to avoid tracking closely related state separately, since it's easy to introduce subtle bugs that way, we should just remove
PDFScriptingManager._pageEventsReady
now.Obviously I should have done this already back in PR #13074, sorry about the churn here!