Switch from schema to clojure.spec

[anthonygalea]

This is a first stab at migrating to clojure.spec. I wouldn't call it ready to be merged but thought it would be a good idea to get feedback early.

[devth]

Whoa, this is awesome! I'll review in detail when I get a chance - hopefully later today. But at first glance looks like the right approach.

[devth]

@anthonygalea looks good so far! I noticed you renamed yetibot.core.adapters.init to yetibot.core.adapters. I'm fine w/ it but what's the intention? Thanks!

[anthonygalea]

This is (I think) a minor improvement in naming with the aim of making things more readable. ::config being a namespaced keyword is :adapters/config instead of :adapters-init/config. This is both shorter and in line with all the other "configs" ex: :logging/config. We don't have :logging-init/config even though it is also used for initialization. In addition here we could have adapters/config instead of ai/adapters-config.

I typically like to work such changes piecemeal into commits as I encounter them. Having said that if such changes bug you let me know and I'll refrain from doing them.

[anthonygalea]

Reviewed pull request, made additional changes and rebased on latest master. Also reviewed corresponding pull request in yetibot.

[devth]

Thanks for the explanation! I agree - :adapters/config is much nicer. And I'm fine with changes like this - just wanted to understand the intent.

I'm trying to figure out why the build is failing. It looks like the db schemas are empty at https://travis-ci.com/yetibot/yetibot.core/jobs/230115009#L250, so the database isn't being properly setup for test cases, which causes subsequent failures.

[anthonygalea]

Fixed, it was a bug in the spec for :schema/spec.

[devth]

Looks great overall - left a couple comments. Also wondering if we should use spec-alpha2 or wait to upgrade.

Differences are documented here but it's supposed to be mostly compatible. I need to read up more on the differences and what it'd look like. Can save it for a future PR if you want.

[anthonygalea]

Thanks a lot for your feedback @devth!
Just addressed the comments.

Regarding spec2, thanks a lot for the link explaining the differences. I would prefer to tackle this in a future pull request if it's ok with you.

[devth]

Regarding spec2, thanks a lot for the link explaining the differences. I would prefer to tackle this in a future pull request if it's ok with you.

Yep, sounds good!

[devth]

Tested locally and everything looks good! @anthonygalea is this good to merge?

[anthonygalea]

Great @devth. Yep good to merge.