Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate old APIs #113

Closed
vemv opened this issue Dec 9, 2021 · 3 comments · Fixed by #118
Closed

Deprecate old APIs #113

vemv opened this issue Dec 9, 2021 · 3 comments · Fixed by #118

Comments

@vemv
Copy link
Collaborator

vemv commented Dec 9, 2021

Currently there are a few ways of running nvd-clojure:

  • As a main program (which can be invoked in a few ways: lein run, clojure -m, java -cp, etc)
    • it optionally accepts a user-provided classpath string as an argument
      • else it's inferred
  • As a Clojure CLI Tool, that accepts a user-provided classpath string as an argument
  • As a Leiningen plugin, which infers the classpath
  • As a program oriented towards tools.deps projects, which infers the classpath by parsing deps.edn.

The APIs that infer a classpath are prone to classpath interference.

Accordingly, they're unsafe APIs, which lead to inaccurate results and increase the support burden in form of github issues.

I'd propose removing them entirely.

I wouldn't be worried about "not breaking APIs" as common between Clojurists, because this project's very goal is to regularly break CI builds 😄 i.e. it's part of the contract that this program will regularly require users to upgrade things if they want to remain safe.

@vemv
Copy link
Collaborator Author

vemv commented Dec 16, 2021

@rm-hull: any thoughts on this one?

@rm-hull
Copy link
Owner

rm-hull commented Dec 16, 2021

Well I guess I’m a bit far away from the codebase these days. I originally wrote it specifically as a leiningen plugin around Jeremy Long’s dependency-check tool. If we take away the lein plugin part, what does the remainder provide over just directly using dependency-check? (I’m not saying this to be flippant btw, this is a genuine ask).

I haven’t done much clojure in the past few years, hence i am a bit out of touch with the whole tooling ecosystem… so to some extent, I would rather you use your judgement with any active community feedback to make the right decision here.

@vemv
Copy link
Collaborator Author

vemv commented Dec 16, 2021

, what does the remainder provide over just directly using dependency-check?

This is a legitimate question.

It seems to me that dependency-check doesn't offer a CLI that accepts a classpath as an argument. Instead it uses "releases" which cannot be consumed from the deps.edn / Lein ecosystems. So it's more stuff to setup, and more opaque as well (which is ironic for a sec tool).

We also can add some custom logic (e.g.), making things more friendly for clojure projects.

Finally there's value in having specific documentation, tests, etc for Clojure projects, else a team using raw dependency-check may not rest assured that it is invoking it in a way that fully makes sense.

so to some extent, I would rather you use your judgement with any active community feedback to make the right decision here.

Cheers ☕️☕️ and thanks for the reply, with all the log4j debacle I think it's good timing to revamp our APIs, readme and announce @seancorfield 's -Ttools addition.

vemv added a commit that referenced this issue Dec 23, 2021
vemv added a commit that referenced this issue Dec 23, 2021
vemv added a commit that referenced this issue Dec 23, 2021
vemv added a commit that referenced this issue Dec 23, 2021
@vemv vemv mentioned this issue Dec 23, 2021
@vemv vemv closed this as completed in #118 Dec 23, 2021
vemv added a commit that referenced this issue Dec 23, 2021
vemv added a commit that referenced this issue Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants