Skip to content

RemovedGetDefinedFunctionsExcludeDisabledFalse: minor tweak for consistency#1162

Merged
wimg merged 1 commit intodevelopfrom
feature/removedgetdefinedfunctionsexcludedisabled-tweak
Jul 18, 2020
Merged

RemovedGetDefinedFunctionsExcludeDisabledFalse: minor tweak for consistency#1162
wimg merged 1 commit intodevelopfrom
feature/removedgetdefinedfunctionsexcludedisabled-tweak

Conversation

@jrfnl
Copy link
Copy Markdown
Member

@jrfnl jrfnl commented Jul 11, 2020

Oops... just saw this after PR #1150 was merged ...

The feature is - at this time - only deprecated. Everywhere else, we throw a "warning" for deprecations and an "error" for removals/new features, so this should have been set to throw a "warning" for now.

Also, as at some point in the future, the feature is expected to be removed, let's change the error code to Deprecated to allow for a second Removed error code once the feature is removed.
Again, that is in line with how it's done in most places in the code base (though a cross-sniff consistency check at some point wouldn't be amiss).
Having different error codes for Deprecated/Removed allows for ignoring deprecations, while still getting the errors when the feature actually gets removed.

@jrfnl jrfnl added Type: chores/QA PR: quick merge PR only contains relatively simple changes PR: ready for review labels Jul 11, 2020
@jrfnl jrfnl added this to the 10.0.0 milestone Jul 11, 2020
@jrfnl jrfnl requested a review from wimg July 11, 2020 21:39
…stency

Oops... just saw this after PR 1150 was merged ...

The feature is - at this time - only deprecated. Everywhere else, we throw a "warning" for deprecations and an "error" for removals/new features, so this should have been set to throw a "warning" for now.

Also, as at some point in the future, the feature is expected to be removed, let's change the error code to `Deprecated` to allow for a second `Removed` error code once the feature is removed.
Again, that is in line with how it's done in most places in the code base (though a cross-sniff consistency check at some point wouldn't be amiss).
Having different error codes for Deprecated/Removed allows for ignoring deprecations, while still getting the errors when the feature actually gets removed.
@jrfnl jrfnl force-pushed the feature/removedgetdefinedfunctionsexcludedisabled-tweak branch from 0d203cb to 6321e80 Compare July 12, 2020 07:36
Copy link
Copy Markdown
Member

@wimg wimg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice pickup :-)

@wimg wimg merged commit 96f361a into develop Jul 18, 2020
@wimg wimg deleted the feature/removedgetdefinedfunctionsexcludedisabled-tweak branch July 18, 2020 22:27
@jrfnl jrfnl removed PR: quick merge PR only contains relatively simple changes PR: ready for review labels Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants