Skip to content
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

Deprecated: Output an informational message for deprecation when no version provided #16774

Merged
merged 3 commits into from Jul 31, 2019

Conversation

@gziolo
Copy link
Member

commented Jul 26, 2019

Description

When there is no options.version param provided deprecated method outputs more relaxed warning message to the Web Console.

The fact that we warn when there is no clear path for removing deprecation is very confusing in my opinion. The change proposed tries to make it less dramatic for 3rd party developers. The idea is that when there is no version provided we are going to support deprecated version forever according to Deprecations document. As soon as we decide to remove it, we should specify the target version for removal and it will automatically make the message stronger.

Before

Screen Shot 2019-07-26 at 18 27 32
Screen Shot 2019-07-26 at 18 24 49

After

Screen Shot 2019-07-31 at 14 03 47
Screen Shot 2019-07-31 at 14 05 42

@gziolo

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2019

@aduth, should we add info to the list of observed console message types in e2e tests?

const OBSERVED_CONSOLE_MESSAGE_TYPES = {
warning: 'warn',
error: 'error',
};

We already cover it for unit tests through jest-console package:

const supportedMatchers = {
error: 'toHaveErrored',
info: 'toHaveInformed',
log: 'toHaveLogged',
warn: 'toHaveWarned',
};

forEach( supportedMatchers, setConsoleMethodSpy );

@aduth

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Hm, I like the updated string, but I feel like we should always warn when something is deprecated. It shouldn't be interpreted as advisory, it should be a motivator to move away from.

@gziolo gziolo self-assigned this Jul 26, 2019

@draganescu

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

I tend to agree with @aduth that deprecations should be a warning indifferent of version. It is one of the ways that pushes people to work on stuff they don't like, all these warnings, while informational messages are not motivating enough :)

Something to add is the word "version" to this string: "%feature% is deprecated and will be removed in version 11"

Deprecated: Output an informational message for deprecation when no v…
…ersion provided

When there is no `options.version` param provided `deprecated` method outputs an informational message to the Web Console rather than warns as previously.

@gziolo gziolo force-pushed the update/deprecated-no-version branch from 2d304fc to 87859a9 Jul 31, 2019

@gziolo gziolo force-pushed the update/deprecated-no-version branch from 87859a9 to 1a7630a Jul 31, 2019

@gziolo

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2019

@aduth and @draganescu - I changed back to console.warn to be used in all cases. I also added the version keyword in the message when appropriate.

@aduth
aduth approved these changes Jul 31, 2019

@draganescu draganescu merged commit 08b9a45 into master Jul 31, 2019

1 of 4 checks passed

Filter opened Filter opened
Details
Filter opened Filter opened
Details
Milestone It Milestone It
Details
Travis CI - Pull Request Build Passed
Details

@github-actions github-actions bot added this to the Gutenberg 6.3 milestone Jul 31, 2019

@youknowriad youknowriad deleted the update/deprecated-no-version branch Jul 31, 2019

@aduth

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Deprecated enhancements got me thinking: #16842

@gziolo

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2019

Deprecated enhancements got me thinking: #16842

Yeah, very cool ideas 💯

gziolo added a commit that referenced this pull request Aug 29, 2019
Deprecated: Output an informational message for deprecation when no v…
…ersion provided (#16774)

* Deprecated: Output an informational message for deprecation when no version provided
When there is no `options.version` param provided `deprecated` method outputs an informational message to the Web Console rather than warns as previously.

* Use warn for all deprecations

* Update unit tests for deprecated method to make them less confusing around version param
gziolo added a commit that referenced this pull request Aug 29, 2019
Deprecated: Output an informational message for deprecation when no v…
…ersion provided (#16774)

* Deprecated: Output an informational message for deprecation when no version provided
When there is no `options.version` param provided `deprecated` method outputs an informational message to the Web Console rather than warns as previously.

* Use warn for all deprecations

* Update unit tests for deprecated method to make them less confusing around version param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.