clj-commons / manifold

A compatibility layer for event-driven abstractions
1.02k stars 106 forks source link

declare core.async as scope "provided" dependency #162

Closed martinklepsch closed 5 years ago

martinklepsch commented 5 years ago

core.async is currently listed in the jar with :scope "test" but it seems that :scope "provided" might be more appropriate since core.async is also an optional runtime dependency.

This is useful for tools that want to analyze Manifolds jar (like https://cljdoc.org) and need to construct a classpath that contains all dependencies needed to load all namespaces.

The better separation between provided and test allows such tools not to add testing related dependencies to the classpath.


PS. There are docs for Manifold on cljdoc. If you want to use those consider adding the following to your Readme (note that previous versions are also available):

[![](https://cljdoc.org/badge/manifold)](https://cljdoc.org/d/manifold)

ztellman commented 5 years ago

I had a different understanding of what "provided" means, but that was totally inferred from context so maybe I got it wrong. Is there some canonical reference on this?

rborer commented 5 years ago

The reference documentation can be found https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope (leiningen uses a Maven library for dependencies management).

provided This is much like compile, but indicates you expect the JDK or a container to provide the dependency at runtime. For example, when building a web application for the Java Enterprise Edition, you would set the dependency on the Servlet API and related Java EE APIs to scope provided because the web container provides those classes. This scope is only available on the compilation and test classpath, and is not transitive.

martinklepsch commented 5 years ago

I think unfortunately there is no scope that perfectly maps to optional dependencies. provided isn’t a perfect match but I think it’s more appropriate than test which I Imagine would be more suited for benchmarking and testing dependencies.

In any case I adjusted the cljdoc analyzer to load some dependencies with scope test, including core.async. I didn’t yet deploy those changes so documentation builds will currently fail for manifold.

ztellman commented 5 years ago

Isn't the problem that you're loading manifold.stream.async? That's only actually loading in manifold.stream if the dependency is available. It seems more appropriate to mark that namespace with some metadata indicating it should be ignored.

EDIT: alternately, you could look up the namespaces I call out in [:codox :namespaces] as the ones that need documenting.

martinklepsch commented 5 years ago

cljdoc doesn't look at build tool specific files like project.clj. In the future there will be ways to exclude files from analysis but for now the only way to exclude namespaces is metadata and metadata requires loading a namespace first (for which all dependencies need to be present).

Anyway it's fixed on cljdoc's end end now so it doesn't matter that much 🙂