Clojurians Log v2

Ah, this might address it: https://github.com/clj-kondo/clj-kondo/blob/master/doc/ci-integration.md#using-ci-specific-configuration

Perhaps also:

--fail-level <level>: minimum severity for exit with error code.  Supported values:
    warning, error.  The default level if unspecified is warning.

yeah you can simply use a file

It would be great to have a flag to only show things that contribute towards a failed exit status. I can't imagine it's very useful to show info level notices in CI.

right, we could introduce a new flag for this: --report-level: minimum level which is reported similar to the other one

issue / PR welcome

(PR optional of course)

I've set a reminder to write an issue at least. I'll take a look at the code (I've got it checked out...) but no promises about a PR 🙂

sure, np. perhaps it's not much more difficult than what the other flag does

I added a ticket: https://github.com/clj-kondo/clj-kondo/issues/2410 I wonder how you feel about making it implicit when you specify the --fail-level flag? That's how I phrased the request. I put in the notice that an explicit --report-level flag would also work for us, but I'm struggling to think of any time I would want to fail at a certain level but still see notices below that. (Other folks might have other use cases, of course.)

I'm happy to rephrase my ticket if that is seen as an unwelcome backwards incompatible change.

I think fail-level should only influence what it says it does, so let's not change that behavior

Thanks for the steer. Issue updated 👍

I'm taking a look to see if I could contribute this option myself. Could it be as simple as adding a filter somewhere near here? https://github.com/stig/clj-kondo/blob/master/src/clj_kondo/core.clj#L262 - maybe in the filter-findings function?

That's probably too early. I'd do it near where the stuff is printed (reported)

Right, so about here: https://github.com/stig/clj-kondo/blob/master/src/clj_kondo/core.clj#L34

I can take a stab at that.

yeah

Hmm, doing the filter at the reporting time means we still get the "warnings: N" part in the summary, but that's probably desired anyway?

we could omit that perhaps according to the report level

Ok, I have an implementation that appears to work:

stig@cci-stig-9c7j1:~/src/clj-kondo/ > clojure -M:clj-kondo/dev --lint - <<< "(defn foo [x] :foo)" 
:1:12: warning: unused binding x
linting took 55ms, errors: 0

stig@cci-stig-9c7j1:~/src/clj-kondo/ > clojure -M:clj-kondo/dev --lint - <<< "(defn foo [x] :foo)" --report-level info 
:1:12: warning: unused binding x
linting took 49ms, errors: 0

stig@cci-stig-9c7j1:~/src/clj-kondo/ > clojure -M:clj-kondo/dev --lint - <<< "(defn foo [x] :foo)" --report-level warning
:1:12: warning: unused binding x
linting took 51ms, errors: 0

stig@cci-stig-9c7j1:~/src/clj-kondo/ > clojure -M:clj-kondo/dev --lint - <<< "(defn foo [x] :foo)" --report-level error
linting took 50ms, errors: 0

stig@cci-stig-9c7j1:~/src/clj-kondo/ > clojure -M:clj-kondo/dev --lint - <<< "(defn foo [x] :foo)" --report-level orrery
[ prints help ]

https://github.com/clj-kondo/clj-kondo/pull/2411 is a draft PR while I look at adding a test

I've added a test (and moved it in a separate commit, as I found a better place to put it 😂 )

There are some test failures in CI that I can't connect to my change, apparently all from this function: https://github.com/stig/clj-kondo/blob/report-level/test/clj_kondo/main_test.clj#L38-L62

(set (drop-while #(not= report-level %)
                                       [:info :warning :error])

I'll take a look at the CI error later

oh I see drop-while , yeah that's not the same

Absolutely open to better ways to write that! It was the second option that came to me. (The first one didn't work 😅 )

one way to look into the CI issue would be to see if you get this error locally as well vs locally with the master branch

will do. I get it locally, but I haven't checked with the master branch. I'll do that now.

if you don't get it on the master branch, it's probably something with the code

Ooh, interesting. I don't see it locally on master, so it is my code. I just can't see how/why (yet)

interesting :)

I've narrowed it down to the test failures going away if I remove the when I introduced here: https://github.com/stig/clj-kondo/blob/report-level/src/clj_kondo/core.clj#L37

maybe there's something weird with the level there. perhaps you can print the level to see if it's nil or something

brb

haha! Yes, the level is :warn in that test, I'll fix that... it's supposed to be :warning I assume?

guess so :)

or is it warn everywhere else except in your code? don't remember ;)

it seems to be :warning everywhere else

ok change it then :)

made one small comment, rest looks good

Thanks. (Hopefully) addressed your comments!

almost. (or (set x) y) will always return (set x) so you need to do something like this:

(if report-level ... (constantly true))

and I think the report-level keyword is only ever used in the context of that function so you can just wrap the two expressions into one let binding

so instead of this:

report-level (some-> report-level keyword)
        report-level? (or (set (drop-while #(not= report-level %)
                                           [:info :warning :error]))
                          (constantly true))

report-level? (if report-level (let [k (keyword report-level)] ...) (constantly true))

Doh! thanks

I'm adding a slight tweak to that to cater for :report-level "piffle"

if the duplication bothers you, you can also write:

I thought of that, but I don't feel strongly about it. Happy to make the change. Whatever you prefer.

I actually don't care about making report-level :piffle work so we could simplify that code a bit. I only care about when not setting report-level the stuff works as before (hence the fallback to constantly true)

aha! yes, that makes sense. I misunderstood your feedback here then: https://github.com/clj-kondo/clj-kondo/pull/2411#discussion_r1799631456

ah yeah makes sense

I would say for a custom log level the min level can't be decided so either all or nothing can be argued for. I think showing nothing is fine (we just don't support it)

Thanks for all the help! Now there's a shiny happy ✅ on that PR.

Thanks a lot!

No, thank you for writing an amazing tool that I rely on at work, and for being cool about helping me with my first contribution 😄