Clojure Code Review: Clj-Slack teardown
This is a blog about the development of Yeller, The Exception Tracker with Answers
clj-slack is a new clojure library for interacting with the Slack rest api.
Recently, whilst building Yeller’s Slack Notifications, I came across this library on github, realized it wasn’t going to work for my needs, and wrote a tiny wrapper around the bit of the API that I needed.
Recently, I noticed that the author had asked for code review of this library. I contacted them and checked if doing that in public would be ok, so here’s a code review of the clj-slack
library:
clj-slack Code Review
So, I’m gonna start with the thing I’m least happy about with this codebase:
Using Environment Variables and Globals for Configuration just isn’t so great
The 12 Factor App Guidelines recommend
Store config in environment variables
Now, this is a great idea and all, but it’s a recommendation for applications, not libraries. clj-slack
sadly uses an environment variable and a global to store the API token it’s connecting to.
(def access-token (str (:slack-token env)))
This makes it really difficult to use for Yeller, and in a whole bunch of other cases where you want to use more than one API token per application.
This is a pattern that (thankfully) is falling out of favor in clojure systems. Stuart Sierra’s even been pushing away from having globals for configuration in applications, a practice I wholeheartedly agree with.
This was the number one thing that stopped me from using clj-slack
for Yeller’s Slack integration, and will prevent a bunch of other uses. If you want to connect to the API on behalf of your users, then you’ll have to do weird dances to make it happen.
Along the same line, not hardcoding the root url of the API will make it much easier to spin up a test server, or even use an behind-the-firewall enterprise install of Slack (if that’s ever a concern, or available etc)
Other, much smaller points
Overall, the code is great - it’s very compact, well layed out etc. I have no real qualms with any of the rest of the code.
Documentation isn’t great yet - there’s no documentation on the params format for many of the functions. However, that’s mentioned in the README, so hopefully it’ll improve in the future.
It might be a good idea, on the documentation front, to link to Slack’s official API docs (which are really great): https://api.slack.com/ - they cover the concepts in more detail than I’d expect in a wrapper library.
Fin
So that’s it. Overall, a nice small well put together library. Only one major gripe, other than that the code looks great, is structured in a sensible fashion (one namespace per REST resource is really something I can get behind). Some work on documentation would be nice, but this library is still in early days, so hopefully that’ll be done soon.
This is a blog about the development of Yeller, the Exception Tracker with Answers.