-
Notifications
You must be signed in to change notification settings - Fork 520
Use context.storageUri for session file (and refactor)
#4088
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
Conversation
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 can't find what this was used for...delete it?
ec15a0a to
1eb62cd
Compare
Note that we seemed to be unnecessarily writing a new debug session file for the `RunCode` and `PesterTests` features; they work fine without that logic as the debugger itself handles the session file.
No idea why we were writing it as the client. Maybe I've totally misunderstood, but everything works as expected without this (and moreover, we delete any existing session file and then wait for it, after passing the path to the server).
As that's the only place they're used.
Since `cwd` can be `undefined` we need to use optional chaining.
1eb62cd to
5f80a6e
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!
I think originally it used to write the session file first and wait for it to be populated... something like that? Can't remember if it was a strange limitation of the time or just accidental over engineering but either way can probably just be dropped 😁
This is a decent cleanup of our session file logic in the client, so it needs a close review. Most important note is that I completely removed
writeSessionFile...and everything works as expected in my testing. Which I think makes sense, as we pass the session file path to the server and expect it to write the session file. I don't know why we had the client ever writing a session file. Furthermore, the only reference I can find to it in the server is with a writer, no reader. 🤷Related to #4071.