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.

Sign up for GitHub

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

Jump to bottom

Set up partial functions ratchet #2974

Merged
merged 13 commits into from
Jun 24, 2022
Merged

Set up partial functions ratchet #2974

merged 13 commits into from
Jun 24, 2022

Conversation

michaelpj
Copy link
Collaborator

@michaelpj michaelpj commented Jun 20, 2022 •
edited by gitpod-io bot
Loading

  • Move the ghcide .hlint.yaml to the top-level
  • Add checks for a number of common partial functions, add exceptions for all existing locations
  • Remove cruft from .hlint.yaml
  • Set the hlint job to fail if there are any errors, ensuring that more instances can't be added without adding exceptions in .hlint.yaml.

@michaelpj michaelpj mentioned this pull request Jun 20, 2022
Open
hasufell
hasufell reviewed Jun 20, 2022
View reviewed changes
.hlint.yaml Show resolved Hide resolved
hasufell
hasufell reviewed Jun 20, 2022
View reviewed changes
.hlint.yaml Show resolved Hide resolved
@hasufell
Copy link
Member

CI error:

  Error: hlint	./ghcide/src/Development/IDE/GHC/CoreFile.hs		200	33	error	Avoid restricted function	Error in isGhcideName in module Development.IDE.GHC.CoreFile: Avoid restricted function ▫︎ Found: "nameModule" ▫︎ Note: may break the code

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from ca9c443 to 46d8dc5 Compare June 20, 2022 12:37
@michaelpj
Copy link
Collaborator Author

Rebased and added exceptions for the new stuff, added unsafePerformIO and fold1 variants.

@michaelpj
Copy link
Collaborator Author

I am very confused about how the changes I've made can affect some test in the func test suite 🤔

@michaelpj
Copy link
Collaborator Author

I think it must be picking up the hlint config.

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from c424232 to 0b5c3ec Compare June 21, 2022 09:35
@michaelpj
Copy link
Collaborator Author

Yep, that was it.

@michaelpj michaelpj requested a review from eddiemundo as a code owner June 21, 2022 10:38
@michaelpj
Copy link
Collaborator Author

How can I be getting ghcide test failures?????

@hasufell
Copy link
Member

How can I be getting ghcide test failures?????

There can be many reasons on windows:

  1. flaky tests, because windows file locking is odd (we get spurious failures in GHC/cabal/ghcup there as well)
  2. github actions environments are in fact rolling-release... so a job that succeeded yesterday may now fail, e.g. due to updated msys2 or other stuff

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from ddb607d to 08771a1 Compare June 21, 2022 13:59
@michaelpj
Copy link
Collaborator Author

I rebased, let's see if it reoccurs.

@michaelpj
Copy link
Collaborator Author

And now it's happy 🤷

@michaelpj
Copy link
Collaborator Author

Anyone like to approve?

@michaelpj
Copy link
Collaborator Author

I'll leave it a day or two in case anyone else has opinions :)

@jhrcek
Copy link
Collaborator

jhrcek commented Jun 23, 2022

Love it. Now I really don't have any excuse for not starting to remove individual partial functions one by one 😄

@michaelpj michaelpj added the merge me Label to trigger pull request merge label Jun 24, 2022
@mergify mergify bot merged commit efcb8e2 into haskell:master Jun 24, 2022
@hasufell hasufell mentioned this pull request Jun 24, 2022
Closed
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
@michaelpj @mergify @hololeap
Set up partial functions ratchet (haskell#2974)
5ebdd8e 
* HLint partial functions

* Delete commented out stuff

* Delete flag-based warnings, unclear they add much

* Delete DA-specific hints

* Delete extra cpp args

* Delete extension options relating to dead build system

* Don't bother trying to restrict CPP for now, it's everywhere

* Make the hlint job fail if there are any errors

* Fix for rebase, add unsafePerformIO and fold1 variants

* Add some more indexing functions

* Try turning off hlint on this test file

* Add .hlint.yaml file for hls-hlint-plugin

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hasufell hasufell hasufell left review comments

@July541 July541 July541 approved these changes

@eddiemundo eddiemundo eddiemundo approved these changes

@pepeiborra pepeiborra pepeiborra approved these changes

@jneira jneira Awaiting requested review from jneira

@Anton-Latukha Anton-Latukha Awaiting requested review from Anton-Latukha

@Ailrun Ailrun Awaiting requested review from Ailrun

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@michaelpj @hasufell @jhrcek @pepeiborra @eddiemundo @July541