-
Notifications
You must be signed in to change notification settings - Fork 188
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
For 2.7.0.0 #310
For 2.7.0.0 #310
Conversation
This module has been kept for backwards compatibility for a long time. It has been unnoficially deprecated via documentation for a long time (since 2014), but has never been given a proper deprecation flag. This flag signals that using this module is an error and should be avoided, something that can easily be missed by not reading the module summary.
We introduced warnings on `String` based functions in 2016. It seems reasonable to upgrade these warnings to `DEPRECATED`. The eventual goal here would be to remove these functions in a major epoch like 3.0.0.0.
Network/Socket/Types.hsc
Outdated
@@ -191,6 +184,8 @@ packSocketType' stype = case Just stype of | |||
#endif | |||
_ -> Nothing | |||
|
|||
{-# DEPRECATED packSocketType "Don't use this" #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get some reasoning in place for these "Don't use this" messages? Why shouldn't we use them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clean up the other instances of "Don't use this"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I changed three "Don't use this".
@eborden Ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks for cleaning those up.
Merged. Thank you for your review! |
2.7
branch is now2.7-old
2.7
was created on2.6
(They point the same commit at this moment)This PR is to recover
2.7
based on2.6
.No deletions. Deprecations only.
Some APIs should be implemented or fixed as discussed in #296 before we release v2.7.0.0.