-
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
Move EOF
/isEOF
from core/parser.js to core/primitives.js
#8003
Conversation
Given the nature of `EOF` and `isEOF`, it seems to me that they really ought to be placed in `core/primitives.js` instead. In general, it doesn't seem great to have to depend on the entire `core/parser.js` file for such simple primitives/helper functions. In particular, while `core/ps_parser.js` is completely separate from `core/parser.js` with regards to its function, it still depends on the latter for just *one* primitive. Note that compared to e.g. PR 7389, this will not reduce the number of dependencies for `core/ps_parser`, however the new dependency IMHO makes more sense.
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/7c948aa133b6f25/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/4776accbedc691d/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/4776accbedc691d/output.txt Total script time: 25.41 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/7c948aa133b6f25/output.txt Total script time: 25.80 mins
Image differences available at: http://107.21.233.14:8877/7c948aa133b6f25/reftest-analyzer.html#web=eq.log |
The Windows failures look like fallout from the recent Firefox 53 update on the bot, and #7898 (comment) ought to explain the changes in the |
/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/a1809562058d16d/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/a1809562058d16d/output.txt Total script time: 2.25 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/3be81002871af08/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/2463158b87e2451/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/3be81002871af08/output.txt Total script time: 25.74 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/2463158b87e2451/output.txt Total script time: 26.13 mins
|
Looks good, thanks! |
Move `EOF`/`isEOF` from core/parser.js to core/primitives.js
Given the nature of
EOF
andisEOF
, it seems to me that they really ought to be placed incore/primitives.js
instead.In general, it doesn't seem great to have to depend on the entire
core/parser.js
file for such simple primitives/helper functions.In particular, while
core/ps_parser.js
is completely separate fromcore/parser.js
with regards to its function, it still depends on the latter for just one primitive.Note that compared to e.g. PR #7389, this will not reduce the number of dependencies for
core/ps_parser
, however the new dependency IMHO makes more sense.