-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
remove custom messages for deprecated/removed options #14396
base: master
Are you sure you want to change the base?
Conversation
Download the artifacts for this pull request: |
Tell users to see interface-changes instead which has more detailed informaion. No point to keep this pointless printing around.
Pointless. Use interface-changes instead.
MP_ERR(config, "This option might have been renamed, removed, or changed semantics.\n" | ||
"See DOCS/interface-changes.rst for possible replacements.\n"); |
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.
Is this really necessary? Seems noisy.
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.
I don't think it's noisy. I will consider changing it if several users complain about 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.
Not sure this is really necessary, but it's fine I guess.
{"gamut-warning", OPT_REMOVED("Replaced by --gamut-mapping-mode=warn")}, | ||
{"gamut-clipping", OPT_REMOVED("Replaced by --gamut-mapping-mode=desaturate")}, | ||
{"tone-mapping-desaturate", OPT_REMOVED("Replaced by --tone-mapping-mode")}, | ||
{"tone-mapping-desaturate-exponent", OPT_REMOVED("Replaced by --tone-mapping-mode")}, |
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.
I don't see how having to scour through interface-changes.rst
is more helpful than this.
I'm also having a hard time seeing what the purpose of this even is. Just removing code for the sake of it?
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.
Please refer to #14382 for the discussion. I believe everything has been said and should give you idea, why this PR exists.
This warning system isn't used in any meaningful way and the messages are pointless when
DOCS/interface-changes.rst
has more detailed and helpful information, so just remove them and tell users to seeDOCS/interface-changes.rst
instead.