Feature Q-codes - Githubissues

Otann / morse

📡 Clojure interface for Telegram Bot API

Eclipse Public License 1.0

257 stars 48 forks source link

Feature Q-codes #56

Open Sandarr95 opened 4 years ago

Sandarr95 commented 4 years ago

So I redid the PR in its own branch.

A couple of things I need some opinion/guidance on:

Is the update to the README.md satisfactory now?
Is it okay to add the dependency on compojure or would you rather see an implementation without an additional dep?
What would make sense to return from the direct-reply handler? I'm not sure about {:status 200}

As for the tests, I've never written those. Interested to learn though, but that will take me a bit. It's probably not correct on the first try.

Otann commented 4 years ago

Is the update to the README.md satisfactory now?

Spot on 👌

Is it okay to add the dependency on compojure or would you rather see an implementation without an additional dep?

I would rather have a morse-compojure module, which can have compojure-specific handlers ;) If you go down that road, I promise to port everything to pedestal even ;)

What would make sense to return from the direct-reply handler?

I think it makes sense, should be enough for Telegram

Sandarr95 commented 4 years ago

I hope the changes are understandable from the commit messages. Looking forward to feedback on the tests, or anything else.

Otann commented 3 years ago

👌

Sandarr95 commented 3 years ago

Tested direct reply as implemented currently against my own project by adding deps.edn in https://github.com/Sandarr95/morse/tree/testing/deps Haven't had the chance to test the req->morse with webhooks yet.