Skip to content
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

Enable tests for ghc 9 and promote ghcVersion check #2001

Merged
merged 26 commits into from Jul 7, 2021

Conversation

jneira
Copy link
Member

@jneira jneira commented Jul 6, 2021

  • Promote GhcVersion datatype and ghcVersion function to ghcide to make it available to all the codebase
  • Use them to replace CPP for code which paths compile for several ghc versions
  • Add GHC90 version and remove GHC84 (as we dont support it anymore)
  • Include test execution for ghc-9.0 and windows and macos (we could remove macos after a succesful execution, as we are not testing it for any version)

@pepeiborra
Copy link
Collaborator

This is very nice, thank you @jneira !!
To prevent the MIN_VERSION macros from coming back, I would also remove the CPP extension pragmas and remove the modules from the allowlist in the ghcide hlint file.

@jneira
Copy link
Member Author

jneira commented Jul 6, 2021

This is very nice, thank you @jneira !!
To prevent the MIN_VERSION macros from coming back, I would also remove the CPP extension pragmas and remove the modules from the allowlist in the ghcide hlint file.

Hmm, afaics the unique module where i could remove CPP language pragma was ghcide/session-loader/Development/IDE/Session.hs and it was not listed in the modules where CPP is allowed:

# Shady extensions
- name: CPP
within:
- Development.IDE.Compat
- Development.IDE.Core.FileStore
- Development.IDE.Core.Compile
- Development.IDE.Core.Rules
- Development.IDE.Core.Tracing
- Development.IDE.GHC.Compat
- Development.IDE.GHC.ExactPrint
- Development.IDE.GHC.Orphans
- Development.IDE.GHC.Util
- Development.IDE.Import.FindImports
- Development.IDE.LSP.Outline
- Development.IDE.Spans.Calculate
- Development.IDE.Spans.Documentation
- Development.IDE.Spans.Common
- Development.IDE.Spans.AtPoint
- Development.IDE.Plugin.CodeAction
- Development.IDE.Plugin.Completions
- Development.IDE.Plugin.Completions.Logic
- Development.IDE.Types.Location
- Main

But hlint should warn about that, so surely i am missing something

@jneira
Copy link
Member Author

jneira commented Jul 6, 2021

I've marked circleci job and the ubuntu test job for 9.0.1 as required
EDIT: Also the build for macos and ghc 8.10.5

@jneira
Copy link
Member Author

jneira commented Jul 6, 2021

We have a succesful run with tests included for ghc-9.0.1 and macos: https://github.com/haskell/haskell-language-server/pull/2001/checks?check_run_id=2997169971
So i am gonna disable tests for it too 😢

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we still need CPP around imports and other non-expression sections

@jneira jneira added the merge me Label to trigger pull request merge label Jul 6, 2021
@jneira
Copy link
Member Author

jneira commented Jul 7, 2021

The test job for windows and ghc-9.0.1 is getting stuck in the last builds 😟

@jneira jneira merged commit 8edb0f7 into haskell:master Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants