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

Feature treat warnings as errors #233

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

refaelsh
Copy link
Contributor

This is just a draft to discuss with you.
Would you be willing to accept such a PR? I will off course take care of all the warnings, or at least try to.

@cdepillabout
Copy link
Owner

I would like to enable "warnings as errors" in CI (but probably not in the actual packages that get released to hackage), and get rid of all the warnings.

One of the problems is that Termonad uses the haskell-gi GTK bindings, which are kind of fragile. The actual haskell-gi family of libraries gets generated with different functions and datatypes depending on what GTK system library the user has installed. End-user programs like Termonad are forced to workaround this by detecting the GTK version, and using CPP to use different codepaths depending on what functions are available.

If you look in Setup.hs, you'll see quite a lot of workarounds for various GTK versions that have changed APIs that Termonad is using.

Most of the current warnings in the code come from trying to find GTK APIs that are stable across versions of GTK (even if they are deprecated in the most recent versions of GTK).

I haven't fixed most of the current warnings because I haven't taken the time to figure out what is the "recommended" API to use in each GTK version.

If you wanted to spend time doing this (annoying?) task, I'd happily merge it in though!

@refaelsh
Copy link
Contributor Author

Oh! I now understand the complexity of the issue. I will try to take a look at the Setup,hs file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants