servant icon indicating copy to clipboard operation
servant copied to clipboard

Resolve build warnings

Open nbacquey opened this issue 2 years ago • 6 comments

This PR solves all current compile time warnings in servant, so that mistakes can be more easily spotted when interacting with the code base.

The only nontrivial fix is the one of "erroring instances" of HasLink, HasClient, and HasServer. Those erroring instances, by construction, do not define all methods of their type class, thus generating warnings. It wouldn't have been reasonable to silence all such warnings in their respective files, because those files contain multiple other instance declarations, and having missing methods warnings for those is useful.

Therefore, I chose to move the erroring instances in separate files (thus refactoring a bit the structure of the code base), and silence "missing methods" warning in those new files only.

The PR may be squashed before merging ; I kept it unsquashed so that modifications to renamed files would be more readable.

nbacquey avatar Mar 17 '22 16:03 nbacquey

@nbacquey Have you noticed that doctests are failing?

ysangkok avatar Mar 18 '22 01:03 ysangkok

@nbacquey Have you noticed that doctests are failing?

Thanks for noticing ; doctests should be fine now

nbacquey avatar Mar 21 '22 14:03 nbacquey

@tchoutri @alpmestan Could I have your position about this PR?

nbacquey avatar Nov 22 '22 16:11 nbacquey

@nbacquey Hi! After a cursory review, I'd like to ask for a changelog entry, as it is my understanding that you add a new public module Servant.Server.TypeErrors that you have split from Servant.Server.Internal. :)

tchoutri avatar Nov 22 '22 16:11 tchoutri

@nbacquey Hi! After a cursory review, I'd like to ask for a changelog entry, as it is my understanding that you add a new public module Servant.Server.TypeErrors that you have split from Servant.Server.Internal. :)

Instead of adding a changelog entry, I preferred to declare Servant.Server.TypeErrors in other-modules instead. Its introduction is just a technicality, users should only use Servant.Server directly to benefit from the custom TypeErrors

nbacquey avatar Nov 23 '22 14:11 nbacquey

Ah I see indeed. :)

tchoutri avatar Nov 23 '22 19:11 tchoutri