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

Added clj-condo configs #27

Merged
merged 1 commit into from
Apr 29, 2023
Merged

Conversation

pratik97
Copy link
Contributor

@pratik97 pratik97 commented Sep 6, 2021

@adambard added configs for clj-kondo linter, as I was getting unresolved-symbol in my IDE while using failjure.

@pratik97 pratik97 force-pushed the add-clj-kondo-configs branch 2 times, most recently from b5b136d to edd1627 Compare September 6, 2021 16:51
Copy link

@ericdallo ericdallo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@@ -0,0 +1 @@
{:config-paths ["adambard/failjure"]}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pratik97 This should be ../resources/clj-kondo.exports/adambard/failjure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@borkdude I think @ericdallo mentioned on Slack that resources folder is already on the classpath(lein adds that), and we don't need to mention the relative path here. It's similar as one of Eric's PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above only works if you invoke clj-kondo with --copy-configs so it will copy resources/clj-kondo.exports/adambard/failjure to .clj-kondo/adambard/failjure which feels a bit unnecessary since the files are already locally available and makes it more difficult to develop the config. Having only one source of truth is better. :config-paths doesn't work with a classpath: it just looks up a directory relative to the config.edn file. So ../resources/clj-kondo.exports/adambard/failjure is what is recommended to use here.
See fulcro's config for comparison:
https://github.com/fulcrologic/fulcro/blob/develop/.clj-kondo/config.edn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@borkdude does this look fine? If so, @adambard can we merge this?

@pratik97 pratik97 force-pushed the add-clj-kondo-configs branch from edd1627 to 1ade903 Compare September 7, 2021 08:38
@borkdude
Copy link

Looks good to me. @adambard Here are some other libraries which include the config similar to this PR, in case you're wondering.

clj-kondo/clj-kondo#1383

@bhurlow
Copy link

bhurlow commented Jul 13, 2022

+1 here

@adambard adambard merged commit 0651a90 into adambard:master Apr 29, 2023
@adambard
Copy link
Owner

Apologies for the long delay, and thanks very much for this!

@frou
Copy link

frou commented Aug 4, 2024

Does failjure's config file for clj-kondo actually make it into the published .jar?

It doesn't seem to for me, i.e. even when using the latest version of failjure (2.3.0) via deps.edn, clj-kondo still seems unaware of how to handle the attempt-all macro, resulting in Unresolved symbol: foo lints.

@TuggyNE, I notice you have a PR #35 refining the config slightly, so I take it that at a basic level things are working for you? Do you use failjure via deps.edn?

@TuggyNE
Copy link

TuggyNE commented Aug 5, 2024

@frou I use Leiningen + Calva for most of my deps. I'm not totally sure whether Leiningen or Calva (or Calva's configuration of clj-kondo or clojure-lsp) is responsible for populating the config files. But it does seem to be using this exact file as merged in this PR and presumably there's no other source for it. (Failjure is not one of the short list of libraries included either in clj-kondo directly or in https://github.com/clj-kondo/configs.)

@borkdude
Copy link

borkdude commented Aug 5, 2024

Does failjure's config file for clj-kondo actually make it into the published .jar?

You can check this with:

$ zipinfo /Users/borkdude/.m2/repository/failjure/failjure/2.3.0/failjure-2.3.0.jar
Archive:  /Users/borkdude/.m2/repository/failjure/failjure/2.3.0/failjure-2.3.0.jar
Zip file size: 69380 bytes, number of entries: 63
...
-rw----     2.0 fat        0 bl defN 23-Apr-29 11:38 clj-kondo.exports/
-rw----     2.0 fat        0 bl defN 23-Apr-29 11:38 clj-kondo.exports/adambard/
-rw----     2.0 fat        0 bl defN 23-Apr-29 11:38 clj-kondo.exports/adambard/failjure/
-rw----     2.0 fat      663 bl defN 23-Apr-29 11:38 clj-kondo.exports/adambard/failjure/config.edn
-rw----     2.0 fat     7256 bl defN 23-Apr-29 11:38 failjure/core.cljc

So it seems so.

But you have to do this to copy the exported configs to the .clj-kondo directory:

mkdir -p .clj-kondo 
clj-kondo --lint "$(clojure -Spath)" --dependencies --copy-configs

You'll get a message about which configs were imported.

Btw @TuggyNE, the failjure jar file also seems to contain AOT-ed Clojure namespaces:

-rw----     2.0 fat     2441 bl defN 23-Apr-29 11:59 failjure/core$when_let_ok_QMARK_.class
-rw----     2.0 fat     1759 bl defN 23-Apr-29 11:59 failjure/core$fn__182$G__177__187.class
-rw----     2.0 fat     2484 bl defN 23-Apr-29 11:59 failjure/core$when_failed.class
-rw----     2.0 fat     2793 bl defN 23-Apr-29 11:59 failjure/core$as_ok__GT_.class

I'm not sure if that is intentional or not.

@borkdude
Copy link

borkdude commented Aug 5, 2024

See also https://github.com/clj-kondo/clj-kondo/tree/master?tab=readme-ov-file#project-setup towards the end.

@frou
Copy link

frou commented Aug 5, 2024

Thank you folks. To inspect the .jar, I had used jar xf to extract it, but apparently that gives different results than listing its contents 🥴

I've got things working by following the instructions https://github.com/clj-kondo/clj-kondo/blob/master/doc/config.md#importing and additionally adding -A:foo into where it says to run clojure -Spath since my Failjure dependency is not in the top-level of my deps.edn file.

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 this pull request may close these issues.

7 participants