search

clj-holmes / clj-watson

clojure deps SCA
Eclipse Public License 2.0
83 stars 9 forks source link

address #103 Streamline dependency(-)check.properties overrides #106

Closed seancorfield closed 2 months ago

seancorfield commented 2 months ago

Signed-off-by: Sean Corfield sean@corfield.org

lread commented 2 months ago

Hi @seancorfield I think this PR supercedes #105? So I'll comment here.

Ideas for deprecating -d

(could be handled as a separate issue)

Option 1: Don't, continue to support. Con: option is confusing Con: we have to maintain and make sure it works

Option 2: Turf it, breaking change. This actually seems ok to me. Con: breaks usage for x number of users. Maybe x = 0? Pro: simplifies our life.

Option 3: Stop documenting the feature and warn if used. Usage and README would not describe feature and warn that it is slated for deletion if used. Pro: gives users time to adapt. Con: burden on us to test/support for a bit longer.

Our new Defaults

  1. For nvd db we are more CI-caching friendly, but there are might be considerations/tips we could add to README. Typically an .m2 CI cache is updated when certain files change, but our cache will now update with NVD db changes when no files have changed. I thought this was important for pomegranate and clj-yaml when caching with nvd-clojure, but we could create an issue to verify.

  2. What does nvd-clojure do? I've had a peek here. Differences:

They override where we don't:

We override where they don't:

Override differences:

Side thought: we'll want to diff on dependency-check.properties for new dependency check releases to see what has changed.

lread commented 2 months ago

I'll see if I can rustle up some info on these differences.

seancorfield commented 2 months ago

Hi @seancorfield I think this PR supercedes https://github.com/clj-holmes/clj-watson/pull/105? So I'll comment here.

Yes, thank you.

Re: deprecation -- I'm in favor of option 3, as long as the CLI lib has support for "hiding" options? We could suggest they migrate to using clj-watson.properties perhaps?

Re: caching -- I assume DC core's default data directory was selected so it would be CI-friendly with Maven caching, so I'm not too concerned about that. We can enhance the README later if folks raise questions about it.

Re: nvd-clojure differences -- I'm comfortable with our overrides that disable various analyzers since those were in clj-watson for quite some time (except a couple which seem like recent adds to DC core but should also be off for clj-watson). I'm curious why nvd-clojure has those additional defaults overridden, esp. disabling openssl? Thanks for following up on those.

lread commented 2 months ago

nvd-clojure settings we don't have:

And differences:

lread commented 2 months ago

Re: deprecation -- I'm in favor of option 3, as long as the CLI lib has support for "hiding" options? We could suggest they migrate to using clj-watson.properties perhaps?

Yes, can do. I'll raise a separate issue.

Re: caching -- I assume DC core's default data directory was selected so it would be CI-friendly with Maven caching, so I'm not too concerned about that. We can enhance the README later if folks raise questions about it.

Somebody (hint, hint) is trying to tell you he might have real-world experience he might want to share that might benefit others. But can be slipped in with #84. So no need for an extra issue.

seancorfield commented 2 months ago

PR updated with analyzer property changes to align with nvd-clojure per your investigation -- thank you!

Somebody (hint, hint) is trying to tell you he might have real-world experience he might want to share that might benefit others.

[adopts very British accent] Good grief, man! Just spit it out! Tell the people what you want them to know...

(PR welcome 🙂 )