-
Notifications
You must be signed in to change notification settings - Fork 71
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
fix(ses): Improve error messages for invalid module records #802
Conversation
f53f46d
to
ecf6fd6
Compare
44ddcc2
to
6b05b91
Compare
37299fa
to
88a7689
Compare
0ac4108
to
a7712cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if (staticModuleRecord === null || typeof staticModuleRecord !== 'object') { | ||
assert.fail( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this needs to be unusually high speed code, no change suggested.
Otherwise
if (staticModuleRecord === null || typeof staticModuleRecord !== 'object') { | |
assert.fail( | |
assert(staticModuleRecord !== null && typeof staticModuleRecord === 'object', |
or
if (staticModuleRecord === null || typeof staticModuleRecord !== 'object') { | |
assert.fail( | |
assert.typeof(staticModuleRecord, 'object', ... ); | |
assert(staticModuleRecord !== null, ...); |
or something along those lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve no reason to believe that the difference would make a perceptible impact on performance, but this is within a performance sensitive region of the Agoric runtime that’s struggling to get close to performance parity with nested evaluate bundles, so I think the code errs on the right side of the balance between succinctness and speed.
a7712cc
to
6c6594c
Compare
3e6f1b4
to
f993be0
Compare
86bd504
to
8937927
Compare
3d38824
to
f7f643e
Compare
ea9d265
to
8f9c5c9
Compare
8f9c5c9
to
5c07c85
Compare
Just some minor improvements I stumbled into in debugging for #794