kmon icon indicating copy to clipboard operation
kmon copied to clipboard

Remove the nodemon dependency?

Open henriksen opened this issue 10 years ago • 20 comments

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

Pros of keeping nodemon:

  • Well established code base
  • Flexible out of the box

Cons:

  • Hard dependency on external tool
  • More work in getting it running

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

henriksen avatar Nov 21 '14 16:11 henriksen

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

spboyer avatar Nov 21 '14 17:11 spboyer

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

henriksen avatar Nov 21 '14 18:11 henriksen

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

luisrudge avatar Dec 09 '14 11:12 luisrudge

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.

davidfowl avatar Dec 09 '14 11:12 davidfowl

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#

henriksen avatar Dec 09 '14 11:12 henriksen

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.

davidfowl avatar Dec 09 '14 11:12 davidfowl

@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.

spboyer avatar Dec 09 '14 17:12 spboyer

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

davidfowl avatar Dec 09 '14 18:12 davidfowl

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?

spboyer avatar Dec 09 '14 18:12 spboyer

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

davidfowl avatar Dec 09 '14 18:12 davidfowl

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

luisrudge avatar Dec 09 '14 18:12 luisrudge

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?

spboyer avatar Dec 09 '14 19:12 spboyer

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

henriksen avatar Dec 09 '14 19:12 henriksen

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

luisrudge avatar Dec 09 '14 19:12 luisrudge

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

henriksen avatar Dec 09 '14 19:12 henriksen

Also, it was a tounge in cheek comment :)

henriksen avatar Dec 09 '14 19:12 henriksen

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.

henriksen avatar Dec 09 '14 19:12 henriksen

@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.

davidfowl avatar Dec 09 '14 20:12 davidfowl

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 avatar Dec 09 '14 22:12 henriksen

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

henriksen avatar Aug 04 '15 10:08 henriksen