search

clj-commons / vizdeps

Visualize Leiningen dependencies using Graphviz
Apache License 2.0
33 stars 4 forks source link

Smallest change possible to get this to work with leiningen 2.8.3 #3

Closed cnuernber closed 5 years ago

cnuernber commented 5 years ago

I don't know if this is backward compatible.

Still calling leiningen internal functions but get-dependencies-memized is now wrapped with a with-binding internal to the leiningen classpath system so this project cannot call into it.

cnuernber commented 5 years ago

No, it now requires an internal variable bound via dynamic binding:

https://github.com/technomancy/leiningen/blob/master/leiningen-core/src/leiningen/core/classpath.clj#L330

danielcompton commented 5 years ago

The bindings change was introduced in https://github.com/technomancy/leiningen/commit/ad6c4bd290d6d59b20e781659d0dd19ca8d50c92, and lein 2.8.0. Making this change would effectively raise the minimum version of Leiningen to 2.7.0, although that may already be the minimum version anyway. See https://github.com/LonoCloud/lein-voom/pull/59 for more details on how this change can break plugins (which were naughtily depending on internal details).

cnuernber commented 5 years ago

OK, that is merged.

In order to do a release, we need to change the project.clj info to change the organization. And then someone needs to type the magic.

Given there are a good number of people who cannot use this right now I suggest we release soon.

danielcompton commented 5 years ago

Yep, I agree doing a release soon is a good idea, I can do that. One thought, while we're changing the group ID, should we also change the artifact name from vizdeps to lein-vizdeps? That clears the space of names for vizdeps to support other build tools too. I think that is a fairly common pattern.

I've pushed the change, but we can revert it if we want to pick a different name.

I've also tested the latest changes out locally and it is working for me with Leiningen 2.8.3 and AdoptOpenJDK (build 11.0.1+13).