-
-
Notifications
You must be signed in to change notification settings - Fork 412
Complete contributing guide #2165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
With the contents of ./CONTRIBUTING.md (mainly the pre commit hook to format)
Included in the main documentation
As building and testing are more important imo
Ailrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update!
| "hooks": [ | ||
| { | ||
| "entry": "stylish-haskell --inplace", | ||
| "exclude": "(^Setup.hs$|test/testdata/.*$|test/data/.*$|^hie-compat/.*$|^plugins/hls-tactics-plugin/.*$)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: we need to update this regex to include up-to-date "excludes" setting in nix config. The current Nix config ignores more files than these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "exclude": "(^Setup.hs$|test/testdata/.*$|test/data/.*$|^hie-compat/.*$|^plugins/hls-tactics-plugin/.*$)", | |
| "exclude": "(^Setup.hs$|test/testdata/.*$|test/data/.*$|test/manual/lhs/.*$|^hie-compat/.*$|^plugins/hls-tactics-plugin/.*$|^ghcide/src/Development/IDE/GHC/Compat.hs$|^plugins/hls-splice-plugin/src/Ide/Plugin/Splice.hs$|^ghcide/test/exe/Main.hs$|ghcide/src/Development/IDE/Core/Rules.hs|^hls-test-utils/src/Test/Hls/Util.hs$)", |
that would be correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow i did not apply this commit 🤦
Co-authored-by: Jan Hrcek <[email protected]>
Co-Authored-By: jhrcek
jhrcek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. LGTM
| An alternative, which only recompiles when tests (or dependencies) change: | ||
|
|
||
| ```bash | ||
| $ cabal run haskell-language-server:func-test -- -p "hlint enables" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this works. It will use the HLS binary in your path, not the development build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems this can make it not use the PATH one:
haskell-language-server/haskell-language-server.cabal
Lines 417 to 419 in 155023f
| build-tool-depends: | |
| haskell-language-server:haskell-language-server -any, | |
| ghcide:ghcide-test-preprocessor -any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that won't help. cabal run does not change the environment, as far as I know.
You can do cabal exec cabal run h-l-s:func-test -- -- -p ... and it will work sometimes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that won't help.
cabal rundoes not change the environment, as far as I know.
It seems so: haskell/cabal#5411 (comment). It works for cabal test but no for cabal run iiuic that comment
You can do
cabal exec cabal run h-l-s:func-test -- -- -p ...and it will work sometimes
It works enough times to update the instructions? are you are referring to the fact cabal exec will not rebuild the executable and you could end using a older version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover with cabal-install-3.8, test arguments will not trigger a rebuild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe cabal exec does trigger a rebuild, but it fails with weird errors in some cases.
Uh oh!
There was an error while loading. Please reload this page.