-
Notifications
You must be signed in to change notification settings - Fork 425
Conversation
src/prepack-options.js
Outdated
debugOutFilePath = "./src/debugger/.sessionlogs/engine2adapter.txt", | ||
}: PrepackOptions): DebuggerOptions { | ||
export function getDebuggerOptions({ debugInFilePath, debugOutFilePath }: PrepackOptions): DebuggerOptions { | ||
// if the debugger is started, the in and out files need to be specified |
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 don't understand this comment in connection with the invariant below which seems to enforce that they do already have to be specified.
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.
This comment just explains why this invariant is here.
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.
Style-wise prefer breaking && checks into multiple invariants (clearer to see which one failed when debugging) and putting comments as invariant messages (2nd arg) if necessary. In this case the comment seems redundant with the invariant.
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.
Updated invariants and removed comment.
1fa1b83
to
4239256
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.
@JWZ2018 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Remove hardcoded default file paths to debugger logs
Remove empty debugger log files