add clj-kondo linting #338
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martinklepsch
approved these changes
Aug 22, 2019
There was a problem hiding this comment.
We have a couple of cases of caught exceptions not being logged. They should be reviewed and a comment added if this is intentional.
We could log these but also it probably doesn't matter much. Feel free to proceed with whatever you think is best 👍
Merge at will 🚀
|
Thanks for reviewing @martinklepsch, mucho appreciated! |
Overview -------- Closes cljdoc#332 - Added clj-kondo with appropriate config to lint cljdoc sources. - For this first run, found mostly unused namespaces and bindings. - Corrected or suppressed all errors/warnings produced by clj-kondo. - Now failing build on circleci should any new errors/warnings crop up. Run the new `script/lint.sh` to lint cljdoc sources. Please continue reading. Unused destructured keys ------------------------ One pattern in cljdoc is the employment of unused destructured keys as means of documenting what is expected for a function. For example, given: `(defn [{:keys [a b c d] :as e}])` cljdoc might not use all of `a` `b` `c` `d` and `e` in the function body. I've deferred the decision of whether or not cljdoc team wants to continue with this practice via script/lint.sh which suppresses this specific case of unused binding. Issues ------ The following should be reviewed before this commit is merged. **unlogged exceptions** We have a couple of cases of caught exceptions not being logged. They should be reviewed and a comment added if this is intentional. See: - [server/api.clj](https://github.com/cljdoc/cljdoc/blob/d54cb58b20d2b4cb1e7c714b3637aa8b89956ce0/src/cljdoc/server/api.clj#L38) - [server/pedestal.clj](https://github.com/cljdoc/cljdoc/blob/d54cb58b20d2b4cb1e7c714b3637aa8b89956ce0/src/cljdoc/server/pedestal.clj#L295) **empty code** An unused function with a big comment and an empty body had me puzzled. Was I right to delete it? See: - [git_repo.clj](https://github.com/cljdoc/cljdoc/blob/d54cb58b20d2b4cb1e7c714b3637aa8b89956ce0/src/cljdoc/git_repo.clj#L219) **oversights?** The comment above an unused binding made me think there might be a programming error - as such I left the code in and temporarily suppressed the linter warning via an underscore prefix. See: - [doc_tree.clj](https://github.com/cljdoc/cljdoc/blob/d54cb58b20d2b4cb1e7c714b3637aa8b89956ce0/src/cljdoc/doc_tree.clj#L89)
incorporate feedback from review with @borkdude. - don't need to build cache then lint, can build cache and lint - include test paths when building cache - decided to listen to :not-empty? warning after all
Upgrade to new version of clj-kondo that allows unused unstructured keys in function defs. This greatly simplifies script/lint.sh which was doing work to suppress these warnings.
address doc_tree.clj issue raised by clj-kondo, now slurping the way we want to.
now logging, rather than swallowing, exceptions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Closes #332
Run the new
script/lint.shto lint cljdoc sources.Please continue reading.
Issues
The following should be reviewed before this commit is merged.
unlogged exceptions
We have a couple of cases of caught exceptions not being logged. They should be
reviewed and a comment added if this is intentional.
See:
empty code
An unused function with a big comment and an empty body had me puzzled. Was
I right to delete it?
See:
oversights?
The comment above an unused binding made me think there might be a
programming error - as such I left the code in and temporarily suppressed
the linter warning via an underscore prefix.
See: