-
Notifications
You must be signed in to change notification settings - Fork 56
Closed
Description
Originally posted by @seanpearsonuk in #4456 (comment)
I'm slightly recoiling on reading this. Let me break it down:
- In the "internal" code (as reviewed today) we see use of
deprecate_argument
in the same context which reads very intuitively. This is a bit more obscure. Does it need to be? deprecated_reason
could be generated. Not generating that message means that each deprecation contains boilerplate that is larger than the non-boilerplate.- Field names here are generally prefixed with
deprecate_
/deprecated_
making the usage verbose (and inconsistent). That contrasts with the terseness and clarity ofdeprecate_argument
. - The structure is deeply nested, making it hard to read and hard to use.
Is the issue that deprecated_version
is missing from deprecate_argument
? Can we add it there and then use deprecate_argument
more widely?
Metadata
Metadata
Assignees
Labels
No labels