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

sbt-revolver like restart feature #523

Merged
merged 4 commits into from
Jun 16, 2017
Merged

sbt-revolver like restart feature #523

merged 4 commits into from
Jun 16, 2017

Conversation

cvogt
Copy link
Owner

@cvogt cvogt commented Jun 16, 2017

No description provided.

@cvogt cvogt changed the title Restart sbt-revolver like restart feature Jun 16, 2017
@cvogt
Copy link
Owner Author

cvogt commented Jun 16, 2017

@gratner I believe you were interested in 0905177?

@cvogt cvogt merged commit 494c302 into master Jun 16, 2017
@cvogt cvogt deleted the restart branch June 16, 2017 03:15
@nafg
Copy link

nafg commented Jun 16, 2017

I wonder if it makes more sense to not have loop and restart separate words? I mean is restart ... "parameterizing" loop or is it really an alternative looping mode?

@cvogt
Copy link
Owner Author

cvogt commented Jun 16, 2017

restart could automatically include trigger looping and "direct" model as well to avoid problems I think. Could you expand on the parameterizing @nafg?

@nafg
Copy link

nafg commented Jun 16, 2017

@cvogt what I meant to say is that the way I parse a command like cbt direct loop restart or cbt loop test is similar to how I parse something like sudo watch time timeout 1 sleep 30:
sudo is a command that takes parameters, its parameters are the command time timeout 1 sleep 30, well watch is a command that takes parameters, its parameters are the command time timeout 1 sleep 30, and so on. Each of the commands to the right could be run on their own, and the command on the left runs them with some modification to their "mode."
So to, direct is a subcommand that takes a further cbt subcommand, like loop restart, and again, loop is a cbt subcommand that takes a further cbt subcommand, like restart.
That's how the proposed syntax "reads" to me, and my argument was that that doesn't really express what's happening. loop isn't "running restart in a modified mode" akin to sudo or watch, in my understanding -- it's really that we're doing an alternative loop mode: The existing loop mode waits for source changes after its subcommand runs to completion, while the proposal is essentially to have a "looping" mode that waits for source changes immediately and then shuts down its subcommand and then runs it again.

I like the idea of shortening the whole thing to one word. After all since currently direct is the only mode supported, it may as well be implied.

While we're on the subject of looping syntax, to go on a bit of a tangent, have you considered instead of using loop doing what seems to be more standard, which is to have a -w / --watch flag (e.g. webpack and tsc, the typescript compiler, do that)? Or at least renaming it to watch instead of loop? It just seems to be the more common naming convention.

More relevantly, I don't know if we should spend the word "restart" on this. Let me explain.

Let's take a step back from "sbt-revolver" functionality and just question how web development would look without it. Would it make sense for cbt to the bash prompt while something is still running? E.g. in sbt with xsbt-web-plugin, you can run jetty:start and be back at the sbt shell, and later do jetty:stop (or just exit sbt). It might be a bad idea to return to bash something still running however. If so then how would one do two things at once (like ask cbt for the classpath while the webapp is running)? We could say they should use regular shell mechanisms, like appending & to background it, or running two terminals, but of course that's not ideal either. Also does the related project to "run a shell for subcommands of any command" have any bearing on this question?

In any case my point is, that if CBT would support running a webapp in the background, then you might want to have a restart command for that.

Actually here's an idea then... name the new command watch. I think loop has a stronger implication that it's "running something [to completion] in a loop [waiting for changes for each subsequent iteration]" while watch has more of a connotation of "being vigilantly on the lookup for activity [and taking action in response]".

(My understanding is that this can handle any subcommand.)

Thoughts?

@cvogt
Copy link
Owner Author

cvogt commented Jun 16, 2017

The existing loop mode waits for source changes after its subcommand runs to completion

correct

while the proposal is essentially to have a "looping" mode that waits for source changes immediately and then shuts down its subcommand and then runs it again.

incorrect. I tried that, but it doesn't work, because it only becomes apparent until after cbt's run which sources will need to be watched. Instead loop actually does the same thing and DOES take restart as an argument that it runs. restart is responsible for spawning the execution in a separate, detached thread and then exiting cbt as usual.

The only new thing is that restart will also write the pid of the process into a file and loop now kills all processes listed in that file before running the task again.

So it really works like your intuition reads the commands if I am not mistaken.

Not 100% sold on loop either, but following your reasoning about intuition watch compile would imply that it is watching compile not watching files. loop follows that inutition here that it loops compile (while also watching files as the trigger).

I like -watch. CBT has room to improve it's argument handling, which I haven't gotten to yet.

Regarding start and stop and restart. Right now killing the previous processes is implemented in the bash script, but maybe we should move that into stop? Then start could be what restart is now and restart could be stop; start. And looping is just separate from that.

@cvogt
Copy link
Owner Author

cvogt commented Jun 16, 2017

I think dropping to bash is fine even when a process keeps running and that solves the question about how to call other cbt tasks in the mean time

@cvogt cvogt mentioned this pull request Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants