-
Notifications
You must be signed in to change notification settings - Fork 1
Description
In #349 we attempt to bump the version of tasty-hedgehog, but it required a bunch of extra workarounds. We decided to wait until various upstreams have cut new releases so we don't have to maintain various hacks. See #349 (comment)_
Probably most of these workarounds will be temporary, but how temporary?
I would expect that they will all be removed when both hlint and hls make new releases. However, this requires hlint to release first and hls to bump their vendored hlint. Since the community is moving slowly wrt adopting ghc 9.2.3 this may take quite a while.
We're not currently held back in any significant way by the our version of
tasty-hedgehog, so these additional workarounds that we've added in this PR are harder to justify,
Agreed
On a related note: how did we end up forking/hacking HLS and/or
hlintin this PR? It's just supposed to be bumpingtasty-hedgehog, right? Are the changes tohlintand HLS required to support the newtasty-hedgehogversion, or are they unrelated and just happened to be added to this PR as part of a general cleanup of developer tooling issues, or is it the case that the changes required for the newtasty-hedgehoghave snowballed and we've ended up at this point?
I have serious concerns about trying to keep the shell's
hlint, HLS'shlint, andpre-commit-hook'shlintall in sync, and I would like to understand why that is now necessary, given that it has not been necessary to date.
Ok. We don't necessarily need the exact same binary, or even the exact same version. However, different versions can give different warnings, giving an inconsistent dev environment and CI.
For background on why this was necessary, see #349 (comment) and the commit messages in #349. To summarise
- We are attempting to upgrade tasty-hedgehog, just as a routine version bump to track upstream
- There was an upstream change deprecating
testPropertyand introducingtestPropertyNamed - If we just upgrade tasty-hedgehog, then everything will work, but we will get a lot of deprecation warnings from GHC (which are errors in CI)
- This is because tasty-discover looks for
hprop_...tests and generates tasty tests containingtestPropertyNamedfor them
- This is because tasty-discover looks for
- There is a recommended change to avoid this, requiring a newer tasty-discover: use custom
tasty_...tests - However, hlint has a warning about camelcasing. Upstream has got a commit on master that updates the hardcoded list of things to ignore for that hint to include
tasty_, but there has not been a release yet. If we don't upgrade hlint we would get a lot of warnings about these names, which make CI fail - Thus we want to upgrade hlint to HEAD. Since we use 3 separate instances of hlint, we need to upgrade them all. We don't need to keep them perfectly in sync, but do need to upgrade all of them to a recent-enough HEAD (technically we only need to update pre-commit-hooks to avoid CI erroring, but for consistency and not overwhelming local development with these reports we update the other two also). These instances are
- shell's hlint: what gets run manually
- hls: this bundles its own copy of hlint, so needs updating separately
- pre-commit-hook: this is run in CI