Skip to content
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

Cleanup the Jest process management #189

Open
marcinczenko opened this issue Dec 11, 2017 · 4 comments
Open

Cleanup the Jest process management #189

marcinczenko opened this issue Dec 11, 2017 · 4 comments

Comments

@marcinczenko
Copy link
Member

Follow up from #179.

@marcinczenko
Copy link
Member Author

In reply to @connectdotz (I moved the discussion form #179 to here as this is now the issue about cleaning up the Jest process management).

ouch, if I made you feel that way, it is my bad. It's great that you feel the code should be improved for simplicity and that's why we love fresh perspectives! I am sure there are many ways to refactor the process and it's hard to fully understand each other's idea via these comments... I just want to share the lesson we learned and balance short-term hack vs long-term fix...

I am not frustrated (yet), I am trying to make sure we are aligned. Time is a limited resource, so I want to get to the point as quickly as possible.

Just because the code is out there, doesn't mean its right or unchangeable ;-)

Did I say anything like that?

when uncertain, I will usually do the backward compatible change, so whoever needs it can get the benefit without breaking others.

That's exactly what I did.

I guess it all depends on what this local abstraction needs to do... If it makes more sense to happen in the external package, then we should fix it there (better encapsulation too) instead of hacking on our side, which is exactly what led us to the process management issue now... just trying to learn from our mistake.

I am starting to have a feeling you are trying to prove something to me and I am not sure this is a constructive way before knowing each other a bit better :). I am not proposing a new abstraction to hack anything, but to create a clean bridge between external dependency, which allows us to be more specific about the contracts. I will not insist of it, so if we decide it does not help, I will be more than happy to delete some code - deleting code feels always good ;).

regarding restarting the process after snapshot update: I see you are trying to call stopProcess() followed immediately by startProcess(). Given stopProcess() will not "complete" right away (like an async operation), the startProcess() should fail to keep the "single-process" mode true. Will it be better to wait after the process is fully stopped before invoking startProcess()? What do you think if we use a promise (or something like that), resolved by debuggerProcessExit, to trigger the startProcess()...? I could be wrong but I think you might not even need to touch the forcedClose or any other part of the system if we can truly serialize the stop/start sequence...

This is the kind of conversation I am interested in. So, yes, if we take the idea of a single process to the extreme, then yes, waiting for the previous process to finish will make the flow easier to understand. We can try that.

How are we going to proceed? Do you prefer that I start with something and then we improve on it via pull request, or you want to propose an initial version?

@connectdotz
Copy link
Collaborator

great, now we are back on the right track. Why don't you take the lead, this was always your project, I think I have done enough damage 😉 Feel free to ping me when you are ready, good luck and have fun!

@marcinczenko
Copy link
Member Author

@connectdotz ok great. I will propose something over couple of days.

@marcinczenko
Copy link
Member Author

I've created a pull-request for the new abstractions for JestProject management.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants