henriksen / kmon

Other
28 stars 5 forks source link

Remove the nodemon dependency? #3

Open henriksen opened 9 years ago

henriksen commented 9 years ago

Right now we're just wrapping nodemon. Should we keep doing that or just implement a file watcher in Kestrelmon?

Pros of keeping nodemon:

Cons:

Perhaps option 3: Make nodemon optional. By doing that we also open up the possibility to wrap other, similar, programs.

spboyer commented 9 years ago

@henriksen I like the implementation of nodemon, but I can add the option to use an alternate to nodemon in the options --watcher nodemon

henriksen commented 9 years ago

Yeah, that's what I'm thinking too. Then you cab use other ones as well. Parameters are just passed through anyway.

luisrudge commented 9 years ago

Can't c# monitor processes? I mean, we shouldn't have to depend on node for this simple task.

davidfowl commented 9 years ago

Why is this using nodemon at all? Can't this just restart the process that it's watching. When you run k --watch, the process suicides when changes are made to appropriate files.

henriksen commented 9 years ago

It's using Nodemon since that was quick and easy for a proof of concept and because Nodemon have some convenient features such x-plat, filtering, ignoring and delaying. But there's no technical reason why we can't do this in C#

davidfowl commented 9 years ago

Not sure that makes any sense as the runtime is the thing killing the process in the case of the KRE. The watching happens by the runtime since it understands what goes into the compilation. The kmon application is responsible for restarting when it dies.

spboyer commented 9 years ago

@davidfowl so are you saying k --watch kestrel should accomplish the same? I didn't get that in testing on OSX in the working directory. kestrel starts, I changed a file in Sublime, but had to restart kestrel to see the change.

davidfowl commented 9 years ago

I'm saying that the only job that kmon needs to do is keep k alive. It doesn't need to watch files

spboyer commented 9 years ago

its either going to be watching k or watching the working directory - either way its watching files correct? the --watch option doesn't seem to be killing k either on file changes.

I see KRuntime/src/Microsoft.Framework.Runtime/DefaultHost.cs#L140 - this is where k should be shutting down correct?

davidfowl commented 9 years ago

That's because the file watcher is busted on mono. But it's being fixed

luisrudge commented 9 years ago

k --watch kestrel does watch for file changes see here. But it just kills the project. What @davidfowl is saying is that kmon should just watch for the termination of the process and restart it, since k --watch does all the file watch

spboyer commented 9 years ago

ok we'll have to look at the adjustment to the kmon cmd when the the file watcher is fixed in mono. @davidfowl is there a Issue we can watch for that?

henriksen commented 9 years ago

Well, there's a reason for using Nodemon, it correctly watches the files on *nix :)

luisrudge commented 9 years ago

That's the whole point: We don't need a file watcher.

henriksen commented 9 years ago

We need it for now, @luisrudge, since k --watch doesn't work on Mono (the way I understand it)

henriksen commented 9 years ago

Also, it was a tounge in cheek comment :)

henriksen commented 9 years ago

The FileWatcher in K is a bit broad. It will kill the server even if you're editing static files like html and css, a bit rough when just a refresh will do.

davidfowl commented 9 years ago

@henriksen False, the file watcher in the runtime watches files as it goes into the compilation. It doesn't die when static files are changed.

henriksen commented 9 years ago

My bad, to me it looked like the line @luisrudge linked to just takes the entire root directory. Must be too late in the evening. Well, that's great then! Time to rewrite!

henriksen commented 9 years ago

Removed in the drop_nodemon branch, needs a bit more testing before pushing to master.