Don’t let MonadZero imply its deprecation warning #68
Conversation
Otherwise instances with a MonadZero constraint in their context hide the warning.
src/Control/MonadZero.purs
Outdated
| @@ -32,13 +33,16 @@ import Data.Functor (class Functor, map, void, ($>), (<#>), (<$), (<$>)) | |||
|
|
|||
| import Prim.TypeError (class Warn, Text) | |||
|
|
|||
| class Deprecated | |||
There was a problem hiding this comment.
Will this work if we have multiple items deprecated? Perhaps we could use a pattern like ‘DeprecatedMonadZero’ if we need to distinguish classes in the future?
There was a problem hiding this comment.
I agree that making this class name more specific could help other issues in the future.
There was a problem hiding this comment.
I've renamed the class. Waiting on CI to build.
There was a problem hiding this comment.
I’m not sure to understand why we should rename the class actually. Don’t the core libraries encourage to qualify imports to disambiguate?
There was a problem hiding this comment.
I meant for within a single module when there are multiple deprecations; here it doesn’t really matter as far as errors go. Though I think it might help make the class more obviously not a generic deprecated class if we name it more specifically for the function or class which is deprecated.
There was a problem hiding this comment.
Oh right, I didn’t pay attention to your first sentence initially for some reason 🤦
What do you think of an IsDeprecated suffix instead? I’m ok with whatever pattern you prefer though.
There was a problem hiding this comment.
That's fine as well -- just something more specific than Deprecated. So MonadZeroIsDeprecated is great, DeprecatedMonadZero is fine, whatever works!
There was a problem hiding this comment.
I like MonadZeroIsDeprecated because it's clearer than DeprecatedMonadZero. The former reads like a statement whereas the latter is left to interpretation.
|
Any idea why CI isn't running? |
|
I've made a mistake with the CI for these libraries, using |
|
@JordanMartinez if you merge master into this branch then CI should run. |
Otherwise instances with a MonadZero constraint in their context hide the warning.