Clojurians Log v2

https://github.com/clj-commons/potemkin/pull/78

I am recommending to disable whitespaces for the review because the formatter also fixed the indenting: https://github.com/clj-commons/potemkin/pull/78/files?w=1

I’ll try to get to it over the weekend. Please ping me on Monday if I didn’t?

Sure, thank you!

I’ve deployed a new version with your PR in it to clojars. 0.4.8 Would you mind taking it for a spin? And if it works, could you write an annoucement in #announcements?

Thanks a lot for your help @ikaraszi!

Sure, absolutely! Thank you very much!

The code works, but I’ve just realized that clj-kondo has a special code to handle potemkin’s import-vars, therefore now clj-kondo throws an error for me on the new usage

PR welcome!

That should be against clj-kondo, and I am not sure how much time it will take for me

I was under the impression that you could bundle clj-kondo settings with a lib.

There are bundled clj-kondo rules but the import-vars macro is explicitly excluded and the definition of that is in clj-kondo. I believe @borkdude knows the reasons behind the decision

you changed the syntax of import-vars?

I added support for renaming, I kept the support for the previous syntax

I kinda hoped import-vars would be dead by now ;)

but clj-kondo issue welcome, I'll see what I can do ;)

Huh! How come? What would be the replacement?

nothing. take a look at tools.build for example. just write normal clojure

anyway, clj-kondo supports import-vars so issue welcome

Yah, I’m curious to the use case where import-vars is a huge benefit.

To re-use the metadata, documentation, parameter naming, etc. and re-export a var with all of those

Yes, I sort of get that, but when is it that you want to do it? I don’t think I’ve ever come across a use for it. I know of one which is here, but the author implemented their own macros https://github.com/expez/superstring/blob/master/src/superstring/core.clj#L42-L43

Basically, the use-case is to let superstring expose all of clojure.string fns under its own ns.

Well in our case we want to re-export only just a subset of vars (one or two)

So for example, there is a db namespace in our application and we are using next.jdbc, and we have a couple of wrapping functions for our convenience for some of the next.jdbc provided functions, like execute! and execute-one!, technically that namespace is the API that our application is using when communicating to the DB layer. We also want to expose other functions like plan (as plan!) but without wrapping.

Right. I guess I’d just wrap. It’s less magical. YMMV

From the repo 🙂

That is what we did, but then the documentation is missing

But that is why I started my thread asking if this is abandoned or what is the state.

So most of the libs under clj-commons are in a state of life support, ie fixing bugs and updating deps and such. The reason for this is to avoid a host of forks. Doing some new features like you did, is totally fine, but not to be expected to be driven by clj-commons themselves.

I’ve added the ticket @borkdude, but TBH I am unsure then we should go this way at all.

I would like to know whether this is a dead end and we should avoid using it or just fell through the cracks

Would a PR adding native deps.edn integration be accepted? That would at least allow forks to be used.

Probably fell through the cracks.

The whole premise of clj-commons is to avoid a proliferation of forks, so it’d be better if we got the PRs through.

@ikaraszi so, you’re interested in https://github.com/clj-commons/potemkin/pull/47 https://github.com/clj-commons/potemkin/pull/77

Yes, exactly!

So, there’s two of them which seem to do some of the same things. Could I ask you to do a review of the two and come up with a recommendation of which one (or both)?

#47 has merge conflicts, but it follows more what has been proposed in #46. #77 is more up to date with a different syntax

I guess it is more like what you guys imagined as the interface for this library. If you ask me I would rather go with the require like aliasing approach. Something like:

(import-vars
  [clojure.string :refer [starts-with?] :rename {starts-with? begins-with?}])

Or if we want to make that shorter, then:

(import-vars
 [clojure.string [includes?] {starts-with? begins-with?}])

But I guess I would prefer the former

IMO if the require like approach would be used then that would require less work on the toolings

If you agree, then I guess I can start working on my proposal as the third PR.

The thing is that this lib is not really actively developed any more, that’s why it’s under clj-commons. I would appreciate a third PR from you.

Alrighty, then I’ll try to do that tonight

Thank you!