Clojurians Log v2

https://github.com/juxt/joplin/pull/97 Whilst I don't think joplin should really have load-string in the first place, and is clearly a commonality to aero. I don't think aero should become a dependency. Any reason you can't use aero to load a config (bypass joplin load string) and then pass that map to the functions?

@otfrom ^

@dominicm No reason whatsoever, just thought it might be a nicer alternative to use aero, and there was a PR made to aero to add 'envf' so that the port would be compatible. But yeah, this is an opinionated PR. Discuss/reject at will 🙂

@dominicm If you're looking at PRs, https://github.com/juxt/joplin/pull/98 is keeping us on a fork at the moment and there'll be a follow-up PR which adds some additional configuration to joplin.dynamodb

@acron I have no idea about DyanmoDB generally 🙃 But I see no reason to bump that. Your configurable table name looked good too.

🙂

RE Aero, concerned that it will basically end up with classpath collisions & exclusions for most people. Aero still has the occasional breaking change added. For example there's some recent discussion about preferring #aero/ref #aero/join #aero/env as the default. I think my design preferences differ from the original authoring of Joplin, I've always found it awkard that it had it's own configuration. That's possibly by design though (that I'm not seeing)

@dominicm That makes total sense. I am happy to close the PR. With your design preferences in mind perhaps the inverse would be a good idea then; removing the load-config function entirely and encourage people to use their own configuration solution (which could be Aero if they like).

@acron My only questionmark over that is whether joplin is this way because of something like lein-joplin.

@dominicm Yes, I suspect that's probably the kink in that plan

Also, the aliases concept becomes a bit harder

lein seed, lein migrate etc

(That might be what you're referring to by lein-joplin)

@acron Yeah. I think on projects I've been on, our migration process was: 1) lein repl 2) (migrate-and-seed :local) (a helper)

So we didn't really run into that.

@dominicm Our use case is similar

@acron Hadn't realised that lein-joplin was deprecated… that makes this much easier then imo. Want to use your own migrate-and-seed? New aliases!

lein run -m app.joplin.alias/migrate is your new alias I would suppose.

@dominicm aliases.clj uses load-config so that'd need some thought: https://github.com/juxt/joplin/blob/master/joplin.core/src/joplin/alias.clj

@acron I think I'd just deprecate the function (never remove it, Hickey–style), no?

@dominicm works for me

jonpither thx for calling me. As you can see I nudged the affected parties. 😄

@dominicm Anything else we need to do to get this merged? 😉 https://github.com/juxt/joplin/pull/98

Or did you wanna talk about Faraday 1.9.0 ?

@jonpither who's clicking buttons and doing releases?

Thatd be you :-)