Conversation
zebot
added
the
ok-to-test
elland
force-pushed
the
lint-also-tests
branch
from
8d348f7 to
4f2938a
Compare
|
Are the hie.yaml changes necessary? They look to me like something that hie-boot or implicit-hie or something else could dynamically generate. And, perhaps more relevantly, something that, if we make this change, developers will have to update in future commits. |
|
@stephen-smith that was updated with implicit-hie, but in any case shouldn't be part of this PR, reverting it 👍 |
elland
force-pushed
the
lint-also-tests
branch
from
674a3fd to
113300b
Compare
fisx
left a comment
fisx
left a comment
There was a problem hiding this comment.
It would have been good to have two PRs: one with the changes to the linting machinery, and one with the mechanical output of make hlint-inplace-all. But since one is so tiny it wasn't a big deal to review it in one gulp.
| @@ -335,7 +335,8 @@ assertCanFind brig from target = do | |||
| let targetHandle = fromMaybe (error "Impossible") (userHandle target) | |||
| get (apiVersion "v1" . brig . path "/users" . queryItem "handles" (toByteString' targetHandle) . zUser (userId from)) !!! do | |||
| const 200 === statusCode | |||
| const (userHandle target) === (>>= (listToMaybe >=> profileHandle)) . responseJsonMaybe | |||
| const (userHandle target) === ((listToMaybe >=> profileHandle) <=< responseJsonMaybe) | |||
There was a problem hiding this comment.
| const (userHandle target) === ((listToMaybe >=> profileHandle) <=< responseJsonMaybe) | |
| const (userHandle target) === (responseJsonMaybe >=> listToMaybe >=> profileHandle) |
(not tested)
There was a problem hiding this comment.
I'm now surprised the linter didn't suggest this itself… I'll test it in a bit.
|
let's hope this settles the topic of hlint once and for all! :) |
4 participants
Added test files to the lint script, they'll no longer be skipped. We can now add these checks to CI and get warnings when it fails.
I've also added an option to bail out of linting if we hit any issues.
The changelog is sadly massive, let me know if it would make sense to do it step by step or if you're fine reviewing it like this.