LibreGaming/matchbot
LibreGaming
/
matchbot
15
1
Fork 1
Code Issues Pull Requests Projects Releases 3 Activity

Improve reliability and quality of the system namespace #1

Merged
seabass merged 4 commits from seabass/matchbot:system-improvements into master 2022-03-26 15:05:32 +01:00
Conversation 3 Commits 4 Files Changed 2 +12 -19
2 changed files with 12 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
README.md
View File

@ -66,18 +66,18 @@ Since `matchbot` uses Clojure's [tools.deps library](https://clojure.org/guides/
You can create, start and stop an instance of the chatbot process with the functions in the `system` namespace:
``` clojure
;; creating a new instance
(def my-instance (atom (system/system)))
;; creating a new instance - an empty Var
(def my-instance nil)
;; starting the instance
(swap! my-instance system/start)
(alter-var-root #'my-instance system/start)
;; restarting the instance
(system/restart my-instance)
(system/restart #'my-instance)
;; stopping and resetting the instance
(swap! my-instance stop)
(reset! my-instance (system/system))
(alter-var-root #'my-instance system/stop)
(alter-var-root #'my-instance (constantly nil))
```
Once you are familiar with nREPL, you can additionally use [tools.namespace.repl](https://github.com/clojure/tools.namespace) to make reevaluating (reloading) your changes easier:

19
src/system.clj
View File

@ -34,12 +34,7 @@
(yaml/parse-stream datafile))
(catch Exception e nil)))
(defn system []
{:config nil
:state nil
:irc nil})
(defn start [system]
(defn start [_]
sm2n marked this conversation as resolved Outdated
sm2n commented 2022-03-23 20:18:29 +01:00
Outdated
Review
Copy Link

start doesn't actually use its input as far as I can tell? I think it'd be better to use _ as the binding name here since that is the case.

`start` doesn't actually use its input as far as I can tell? I think it'd be better to use _ as the binding name here since that is the case.
seabass commented 2022-03-23 20:54:44 +01:00
Outdated
Review
Copy Link

I was imagining that, in the future, this could be used to pass custom options to a single instance for development purposes, and also eventually for the main function to read the config and pass it in rather than system/start doing it by itself. Do you have any preference here? For now, yes, I agree it would be better to use _ as the binding name.

I was imagining that, in the future, this could be used to pass custom options to a single instance for development purposes, and also eventually for the main function to read the config and pass it in rather than `system/start` doing it by itself. Do you have any preference here? For now, yes, I agree it would be better to use `_` as the binding name.
sm2n commented 2022-03-23 21:14:57 +01:00
Outdated
Review
Copy Link

No, I don't have a preference here. I think what you said makes sense with regards to custom options etc. Feel free to mark this resolved.

No, I don't have a preference here. I think what you said makes sense with regards to custom options etc. Feel free to mark this resolved.
seabass commented 2022-03-24 23:08:47 +01:00
Outdated
Review
Copy Link

Resolved in commit 91cbdb69da.

Resolved in commit 91cbdb69da.
(let [config (load-config "config.yaml")
state (atom (load-state (:data-file config)))
irc (irc/new-irc-connection state config)]
@ -55,14 +50,12 @@
(deref (:state system)))
(irclj.core/quit (system :irc))))
(defn restart [system-atom]
(defn restart [system-var]
(do
(swap! system-atom stop)
(reset! system-atom (system))
(swap! system-atom start)))
(stop (deref system-var))
(alter-var-root system-var start)))
sm2n marked this conversation as resolved Outdated
sm2n commented 2022-03-23 20:19:27 +01:00
Outdated
Review
Copy Link

No reason for this either, I think? As the system value isn't used for start, and running stop multiple times is safe.

No reason for this either, I think? As the system value isn't used for `start`, and running `stop` multiple times is safe.
seabass commented 2022-03-24 23:10:00 +01:00
Outdated
Review
Copy Link

I've just made commit 91cbdb69da - is this what you had in mind?

I've just made commit 91cbdb69da - is this what you had in mind?
(defn -main [& args]
(let [main-system (atom (system/system))]
(swap! main-system system/start)
(let [main-system (system/start nil)]
(.addShutdownHook (Runtime/getRuntime)
(Thread. (partial swap! main-system stop)))))
(Thread. #(stop main-system)))))