technomancy/slamhound

Should call (shutdown-agents) at end

seancorfield opened this issue · 8 comments

Since Slamhound loads (and evals) user namespaces, it should call (shutdown-agents) when it is done in case something in the user namespace has started up threads. Otherwise Slamhound will "hang" for about 60 seconds at the end of its run.

Calling shutdown-agents isn't safe to do in the repl because once you do that you can't start them back up again. It might be OK to put in -main, but it would need a clear disclaimer that it's not for repl use.

guns commented

@technomancy

I added some bad advice in the README to call -main to reconstruct namespaces in place from a repl. I did this primarily to avoid users having to construct a java.io.File object to pass to swap-in-reconstructed-ns-form. Would you object to adding another function to slam.hound for easier repl usage, or at least extend the swap-in-reconstructed-ns-form to convert string arguments into File objects?

@technomancy Yes, I was only expecting it to be called on the -main path - and was not expecting that to be used from the REPL.

@guns I wouldn't say it's currently bad advice to call -main from the repl; just that it would be once this gets added. Changing swap-in-reconstructed-ns-form to accept a string sounds fine. It might be nice to have a shorter name for repl-based usage too.

guns commented

It might be nice to have a shorter name for repl-based usage too.

I take back this suggestion; it seems superfluous since users should be encouraged to use REPL plugins (eventually via nrepl/op).

Changing swap-in-reconstructed-ns-form to accept a string sounds fine

Okay, then I will add this, add shutdown-agents to -main, and amend the README.

guns commented

Okay, it's done. Thanks again!

Cool. I look forward to testing 1.5.4 when it is released to Clojars. I'll test a snapshot from Git for now.

Thumbs up on this - thank you!