This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2022-09-01
Channels
- # adventofcode (2)
- # announcements (3)
- # babashka-sci-dev (79)
- # beginners (76)
- # biff (2)
- # calva (32)
- # cider (2)
- # clj-kondo (42)
- # clj-on-windows (17)
- # clojure (28)
- # clojure-belgium (1)
- # clojure-berlin (1)
- # clojure-europe (95)
- # clojure-nl (4)
- # clojure-norway (4)
- # clojure-uk (5)
- # clojurescript (27)
- # conjure (5)
- # cursive (3)
- # data-science (16)
- # datomic (67)
- # graalvm (12)
- # hyperfiddle (36)
- # jobs (3)
- # jobs-discuss (1)
- # kaocha (2)
- # klipse (1)
- # leiningen (28)
- # lsp (16)
- # luminus (3)
- # malli (10)
- # nrepl (3)
- # off-topic (57)
- # other-languages (18)
- # re-frame (4)
- # reitit (8)
- # releases (1)
- # remote-jobs (1)
- # scittle (4)
- # shadow-cljs (7)
- # test-check (1)
- # tools-deps (4)
- # vim (11)
- # xtdb (25)
How interested would you be in auto-generated docs and default configuration for clj-kondo? I know you're not interested in splitting out the implementation of the rules, but I'm thinking that we could move the config info and documentation into individual files and then merge them as part of the build or release pipeline
haha yes it would, which is why I'm asking before doing anything dumb 😇
I'll give it a go, see how fruitful/fruitless it is, and let you know!
I've opened a draft PR to demonstrate colocating linter config and defaults, and their documentation: https://github.com/clj-kondo/clj-kondo/pull/1805. I'd like to get feedback from other clj-kondo hackers and devs as well as @borkdude himself.
I went with a single file instead of multiple files, for ease of initial implementation, but splitting them would be quite easy.
I think moving to multiple files is probably best, but then we'd want to bring in parse-edn-metadata-headers from markdown-clj or write our own so we could have edn metadata at the top and raw text examples below:
{:name "Cond-else"
:keyword :cond-else
:default-level :warning
:description "Prefer `:else` as `cond` default branch."}
;;BAD
(cond
(odd? (rand-int 10)) :foo
:default :bar)
;; GOOD
(cond
(odd? (rand-int 10)) :foo
:else :bar)
Your use of good/bad in the above example reminded me https://github.com/bbatsov/clojure-style-guide/issues/216#issuecomment-746682308.
Sure, the wording can be changed. I just mean to have multiple examples for what is and is not allowed by a given rule. Here's an example from eslint, the de facto javascript linting engine: https://eslint.org/docs/latest/rules/for-direction
> Examples of incorrect code for this rule:
And here's how rubocop presents their linter examples: https://docs.rubocop.org/rubocop/1.26/cops_style.html#styleandor
I’m hoping that by having more space for examples, we could list the alternatives and possibilities for each configuration, so the goal isn't to say definitively “X is good, Y is bad”, but “with linter L configured like this, X is allowed, Y is disallowed”
Thanks for the link, I think @U05254DQM is right to avoid “good”/“bad” language
Funnily enough, it was probably bozhidar who wrote both the clojure and rubocop text.
Not suggesting there is a good or bad wording (see what I did there? :drum_with_drumsticks:).
Just remembering some folks thoughtfully talking about it.
Perhaps “Lint issue:” vs “Lint free” or something like that?
Yeah, I’m not super worried about the specific wording lol. I just want to reduce friction for writing linter documentation, make it slightly easier and fix things like “linter X doesn’t have docs on accident”
• Problem statement. What I'm missing in this PR is a good problem statement: why are we making a big change, to solve what problem again? Big changes come with risk, so they should be justified
• Performance. I haven't looked deeply yet at this PR but one thing I'm always concerned about is performance regressions or changes that affect startup time. Writing a literal map in the impl/config namespace is likely going to be faster than having to re-construct it from a string that has to be parsed from somewhere else on the classpath. And moving this stuff elsewhere also would break the clj-kondo pod (binary) so that would have to be fixed too (can be done, but something to remember).
A few thoughts
◦ measure the time difference between loading the current clj-kondo and the clj-kondo with this PR. Also mind the laziness (I think I saw a map
somewhere in the reconstructed version of the map): laziness often gives a skewed performance benchmark. So maybe replace those lazy things with mapv
etc.
◦ measure how much time it takes to parse and reconstruct the map with a (time ..)
expression
◦ Consider leaving the old map where it is and use that as the source of truth, while adding things to it from other places when generating docs
◦ Adding things to the "source of truth" map possible makes it more expensive to merge and merging configs is a thing that happens a lot. So not adding things to the old map when not generating documentation is preferred.
Thanks for the thoughts, Michiel, I'll write some stuff up and see what changes
So, I think your approach has benefits, but I also would like it if there would be no impact on the current way of doing things: minimal map literal (merge and loading time). Would it be possible to build the documentation stuff as another map + a deep-merge with the config in impl/config?
with the .edn
file, on my 2018 mac book pro, it took roughly 2ms to load, read-string
the edn file, and perform the transformation. moving it to a clj
file makes it cost "nothing" to load, and the transformation step is roughly 0.05 ms.
seems good enough then. btw, you can get a tiny bit of perf win if you don't use :keys
but just use (:foo ...)
As a last step, can you update the dev.md
docs to indicate what people should do when they want to add docs?
i'm also trying to figure out exactly how to write out what I want/hope to achieve here, which I know is a bad sign for merging, but it's more that it's harder to write about "dev experience" than "user experience". roughly, the process for writing a lint involves touching a lot of places: the specific part of the code that performs the check, the tests, the default config map, and the docs. The code and tests are to be expected, but the default config map and the docs are places where it's easy to make mistakes. For example, the docs disagree with the default config map on the default warning level for one of the lints (i didn't note which one, just used the one in the code). Both the config map and the docs are not in alphabetical order. The docs are missing some lints. Etc. This raises the burden of writing and maintaining the lints because all of these things have to be coordinated, and there's no automated way to know if they're in sync.
Yes, right now I have it in a .clj
file. lol sorry for writing all that out and missing your other messages
I think it's a little harder to use as a .clj
file, but that's mostly due to how we don't have multi-line strings in Clojure.
Btw, I didn't get the ".clj has not multiline strings" comment, how is that different from .edn?
It’s not, I just found it harder to get indentation correct with additional indentation from the def
form and quoting. Purely a personal preference.
You know what. Maybe just a script that checks if all linters have documentation should be enough. And alphabetically sorting the entries (this is not a must for me personally). I prefer writing .md than in .edn.
This is where "start with a problem statements" comes in. This PR starts with a solution without a problem
If the problem is: you might miss writing docs for some linters, there should be various solutions and then a PR
If that suits you better then I can do that. I think the problem is bigger than "missing docs for some linters", but like I said earlier, it's much harder to quantify "poor dev ex" so I understand. I agree that writing .md
is much nicer than writing .edn
and it's something I'm frustrated by in this attempt. I'll close this PR and open one focused on adding missing docs and sorting the documentation.
I'm gonna keep poking at this because I think it's worthwhile, but I'll wait until I have a much better solution. Thanks for all the feedback, I really appreciate your patience and continued help
@UEENNMX0T I find working out what problem I am trying to solve in a GitHub issue (or a GitHub discussion) often helps me out. I know I appreciate this kind of thing too on projects I maintain.
hah Yeah, I generally do that first but jumped the gun here. Oops!
I've opened a draft PR to demonstrate colocating linter config and defaults, and their documentation: https://github.com/clj-kondo/clj-kondo/pull/1805. I'd like to get feedback from other clj-kondo hackers and devs as well as @borkdude himself.