Fork me on GitHub
#clj-kondo
<
2019-12-14
>
borkdude12:12:09

@qythium You can re-run your PR using the following: Go to https://circleci.com/gh/yuhan0/clj-kondo/27 and press "Rerun workflow" Meanwhile I've added some notes on PRs here: https://github.com/borkdude/clj-kondo/blob/master/doc/dev.md#pr

qythium13:12:58

Pressing "rerun workflow" still results in the same error

borkdude13:12:25

can you merge with master and push that?

qythium13:12:06

It has to be a force-push, is that alright?

borkdude13:12:32

why does it have to be a force push?

qythium13:12:52

oh sorry, I rebased on master instead of merging

borkdude13:12:41

I think CircleCI gets confused when you force push a commit when another is still running, or something along those lines

borkdude13:12:42

Pushing a totally new commit should fix it

qythium13:12:03

Maybe I'm missing something, but I could only get my branch to run on Circleci by editing the yml config file and pushing a temporary commit

qythium13:12:30

the force push was to remove that commit after making the PR, sorry about the trouble it caused

qythium13:12:30

same error.. should I make a new branch and try opening another PR?

borkdude13:12:39

yeah, let's just do that

borkdude13:12:49

btw, the spec for get is obviously wrong if you look at https://github.com/clojure/clojure/blob/653b8465845a78ef7543e0a250078eea2d56b659/src/jvm/clojure/lang/RT.java#L758 anything may implement the ILookup interface, that's why it's spec'ed as any in speculative: https://github.com/borkdude/speculative/blob/master/src/speculative/core.cljc#L202

qythium13:12:02

ah, my mistake

qythium13:12:26

should I change it to :any or remove the commit altogether?

borkdude13:12:46

maybe make a note there, so it will be left untouched

borkdude13:12:19

keys nilable associative is also wrong since (keys (filter ... {:a 1})) works

borkdude13:12:35

that's why it's seqable

borkdude13:12:07

so only the x -> nilable/x things are good to add I think

qythium13:12:26

hmm, alright

borkdude13:12:08

yeah, it's complicated. the specs in speculative already dealt with a lot of examples from the wild, so its good to look at those first

qythium13:12:27

so like the type system isn't strong enough to capture those functions

qythium13:12:50

should get be left unspecced then? since it's essentially an any->any function

borkdude13:12:17

the type of keys is Seqable[MapEntry] if you would have higher kinded types

borkdude13:12:40

but we don't have that, it's a pretty weak typesystem that only screams at you if something is clearly wrong

qythium13:12:37

alright, pushed with comments

borkdude13:12:02

Did you do any changes to .circleci/config.yml? I think that might be the reason why the JVM tests aren't passing

qythium13:12:28

not on the PR branch

borkdude13:12:16

normally the script looks at CIRCLE_PR_USERNAME, but somehow that's not set

borkdude13:12:20

for example, if you look at another PR: https://github.com/borkdude/clj-kondo/pull/651 it just works

borkdude13:12:39

maybe you should try to unfollow the project on circleci again, because it seems the project isn't running the PR branch under my name

borkdude13:12:50

confusing, this

qythium13:12:00

yeah sorry, it's my first time on circleci and I might have messed things up without knowing how

borkdude13:12:08

@ it seems like his PR isn't running on my account somehow: https://circleci.com/gh/borkdude/clj-kondo

borkdude13:12:45

also it seems his macOS step isn't running

qythium13:12:07

I think that was because I'm on the free plan and macOS jobs are only for paid plans

borkdude13:12:53

I'm also on the free plan

borkdude13:12:42

@qythium can you delete clj-kondo from your circleci account and then do another commit?

borkdude13:12:06

(dummy commit to re-trigger circleci)

qythium13:12:26

I've already "unfollowed" the project, is deleting a separate thing?

borkdude13:12:50

it seems like it's now correctly building

borkdude13:12:44

so I probably gave the wrong tip of following the project

qythium14:12:00

Interesting, but that would make it complicated for contributors to run and test things like the performance script on their end without opening a PR

qythium14:12:01

or creating a local Docker container and running Circleci there, from what I gathered

qythium14:12:43

Actually my main motivation for looking into clj-kondo's type checker was to have it warn me about using get on vectors that had accidentally been converted to lazy seqs - I found myself making that error a few times resulting in hard-to-trace bugs because it would propagate nils silently

qythium14:12:57

It's good to know that I can choose to override it at a local config level, even if :associative is technically too narrow

borkdude15:12:26

as a last step (and I hope you're not getting annoyed now): it would be good to add a couple of unit tests for the things you changed, so they will lock down the changes you made

borkdude15:12:23

pushed a test myself now

qythium15:12:04

oops, only just saw this

qythium15:12:20

thanks for your time! I wasn't annoyed at all :)

borkdude15:12:01

and thank you for your contribution 🙂

borkdude12:12:26

And I've added a script in script/diff which you can use to detect regressions

kevin.van.rooijen12:12:02

Hey, is it possible to add linter configuration to a library? For example I have my main project, and a library, If I add this to my main-app/.clj-kondo/config.edn it works. But if I add it to my-lib/.clj-kondo/config.edn , and then add my-lib to the dependencies of main-app it doesn’t work. I couldn’t find it in https://github.com/borkdude/clj-kondo/blob/master/doc/config.md

borkdude12:12:31

There is an issue for this: https://github.com/borkdude/clj-kondo/issues/559 Please read it and feel free to share your thoughts there

kevin.van.rooijen12:12:46

Thanks! I’ll take a look

borkdude21:12:32

Clj-kondo v2019.12.14 Unresolved namespace linter and 13 other enhancements/fixes! This might be the last release for 2019. Thanks you all for the great feedback and contributions. https://github.com/borkdude/clj-kondo/releases/tag/v2019.12.14