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

Delete HLS installer scripts #2773

Merged
merged 3 commits into from Mar 11, 2022
Merged

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Mar 10, 2022

Prefer ghcup for installations from source.

Since in the last release, the deprecation of this script has been announced, I figure it is time to pull the plug on it now, since ghcup covers all of its use-cases nicely.

One small regression: We no longer support stack for from source installations.
However, I bet it would be trivial to add something like that to ghcup if desired.

docs/installation.md Outdated Show resolved Hide resolved
@@ -83,147 +93,6 @@ pkg install hs-haskell-language-server
to install it. At the moment, HLS installed this way only supports the same GHC
version as the ports one.

## Installation from source
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could continue to describe installing from source with the same caveats as installing from Hackage, below. That is, it only builds a single binary for the current GHC. It would be a very short section, though: cabal install haskell-language-server!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since it doesn't include the proper suffix, not sure if we want to do that. The wrapper might pick it by accident even if the version doesn't match. But no strong opinion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The same is true of installing from Hackage, which we do document. That's why I'm suggesting we could document it with the same caveats.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair point, will change it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I restored the docs with notice, but tried to minimise it in order to be not redundant.

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

You deleted exe, which is the main HLS executable 😅

@michaelpj
Copy link
Collaborator

Also there are a few references to the install folder in workflow files that could be purged.

@fendor
Copy link
Collaborator Author

fendor commented Mar 10, 2022

You deleted exe, which is the main HLS executable 😅

Ah, I thought I misclicked, but couldn't tell :D anyway, CI and you caught it ;D

@hasufell
Copy link
Member

You deleted exe, which is the main HLS executable sweat_smile

Cleaning up code is always good 😁

fendor and others added 2 commits March 11, 2022 10:48
Prefer ghcup for installations from source.
Co-authored-by: Julian Ospald <hasufell@posteo.de>
@wz1000 wz1000 added the merge me Label to trigger pull request merge label Mar 11, 2022
@mergify mergify bot merged commit a9ae9d1 into haskell:master Mar 11, 2022
July541 pushed a commit to July541/haskell-language-server that referenced this pull request Mar 30, 2022
* Delete HLS installer scripts

Prefer ghcup for installations from source.

* Use simpler ghcup syntax

Co-authored-by: Julian Ospald <hasufell@posteo.de>

Co-authored-by: Julian Ospald <hasufell@posteo.de>
Co-authored-by: Michael Peyton Jones <me@michaelpj.com>
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

4 participants