Add restart core. Closes #1548

[juliangruber]

Closes #1548

[bajtos]

This looks like a disruptive change to me. Did you check that the caller of setup can handle the case when setup never returns?

IIRC:

• now: setup returns immediately after starting Station Core process, the process runs in the background
• your change: setup waits for the process to complete

If we decide this is the direction to go, then I think we should rename start to something like run or runOnce, considering that the function no longer starts the process in the background but instead waits until the process exits.

[juliangruber]

This looks like a disruptive change to me. Did you check that the caller of setup can handle the case when setup never returns?

IIRC:

• now: setup returns immediately after starting Station Core process, the process runs in the background
• your change: setup waits for the process to complete

If we decide this is the direction to go, then I think we should rename start to something like run or runOnce, considering that the function no longer starts the process in the background but instead waits until the process exits.

Yes, core.setup() is the last call performed by run() in main/index.js. We could change its name as to not break consistency with the other functions called setup(), what do you think about start()? run() also works. runOnce doesn't work, as the function restarts Core inside its body.

[juliangruber]

This looks like a disruptive change to me.

Are you saying we should find a different way to restart it?

[bajtos]

Yes, core.setup() is the last call performed by run() in main/index.js. We could change its name as to not break consistency with the other functions called setup(), what do you think about start()? run() also works. runOnce doesn't work, as the function restarts Core inside its body.

Thank you for checking!

Based on what you wrote, I think it will be best to rename setup to run.

This looks like a disruptive change to me.

Are you saying we should find a different way to restart it?

I'd like to be careful and make sure we understand the ramifications of this change.

It looks "easy" and innocent at first sight, but changes the behaviour in subtle ways that can easily introduce bugs - at least, that's what I remember from how we were iterating on lib/zinnia.js in Station Core.

[juliangruber]

Yes, core.setup() is the last call performed by run() in main/index.js. We could change its name as to not break consistency with the other functions called setup(), what do you think about start()? run() also works. runOnce doesn't work, as the function restarts Core inside its body.

Thank you for checking!

Based on what you wrote, I think it will be best to rename setup to run.

I added run in addition to setup, so that run doesn't modify ctx, which typically only the setup methods do.

This looks like a disruptive change to me.

Are you saying we should find a different way to restart it?

I'd like to be careful and make sure we understand the ramifications of this change.

It looks "easy" and innocent at first sight, but changes the behaviour in subtle ways that can easily introduce bugs - at least, that's what I remember from how we were iterating on lib/zinnia.js in Station Core.

I agree, that migration was quite tricky!

[bajtos]

👌🏻