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

Add deprecation warnings #891

Merged
merged 2 commits into from
May 20, 2016
Merged

Add deprecation warnings #891

merged 2 commits into from
May 20, 2016

Conversation

tysongach
Copy link
Contributor

@tysongach tysongach commented Apr 15, 2016

Introduce remaining deprecation warnings for features that are removed in 5.0.0.

This borrows heavily from how we deprecate features in Neat using Sass @warn: thoughtbot/neat@5c337cc

One key difference from Neat is that we only want to use this mixin for
deprecations, not all warnings. The reason is because with CSS, it's not always
a straightforward task to upgrade libraries or frameworks; particularly major
releases. So we offer a toggle to globally disable deprecation warnings, but for
other meaningful warnings we need to throw, we still output those.

People can disable deprecation warnings by setting a variable to false:

$disable-bourbon-deprecation-warnings: true;

One problem with this system is that we cannot use these deprecation mixins to
deprecate a function. This is because Sass doesn't allow you to include a mixin
in a function. For now, since we only have a few functions up for deprecation,
we can live with this and manually write warnings for those.

The plan is to merge this and release it as a 4.3 release to solely introduce
these warnings.

@tysongach tysongach changed the title WIP: Add deprecation warnings Add deprecation warnings Apr 17, 2016
@tysongach tysongach force-pushed the tg-5-0-deprecations branch 3 times, most recently from 8465a83 to 9ba0b5e Compare April 18, 2016 14:08
@charset "UTF-8";

/// Throws Sass warnings to announce library deprecations. People can disable
/// these using the `$disable-bourbon-deprecation-warnings` variable.
Copy link

Choose a reason for hiding this comment

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

How about:

/// these by setting the `$disable-bourbon-deprecation-warnings` variable to `true`.

Choose a reason for hiding this comment

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

There is also something funny to me about the double negative here. What if we were to name the variable show-bourbon-deprecation-warnings or output-bourbon-deprecation-warnings or something that will get us to a place where we are more direct with false turning it off, and true turning it on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, great point. I debated this because this current language aligns with Neat’s implementation (suite consistency), but the double negative is indeed a bummer. output-bourbon-deprecation-warnings is much more clear and straightforward.

@jasonramirez
Copy link

Whew that was a long one. Code looks good. I have a particular comment about the double negative variable $disable-bourbon-deprecation-warnings and perhaps turning into an explicit positive. Other than that 🍔

@tysongach
Copy link
Contributor Author

Thanks, @jasonramirez!

@tysongach tysongach added the v4 label May 20, 2016
@tysongach tysongach added this to the 4.3.0 milestone May 20, 2016
Tyson Gach and others added 2 commits May 20, 2016 17:00
This borrows heavily from how we deprecate features in Neat using Sass @warn:
thoughtbot/neat@5c337cc

One key difference from Neat is that we only want to use this mixin for
deprecations, not all warnings. The reason is because with CSS, it's not always
a straightforward task to upgrade libraries or frameworks; particularly major
releases. So we offer a toggle to globally disable deprecation warnings, but for
other meaningful warnings we need to throw, we still output those.

To deprecate a feature, you include the `_bourbon-deprecate` mixin to throw a
Sass warning, providing a detailed message that gives context.

We offer a variable to allow people to disable deprecation warnings from being
output.

One problem with this system is that we cannot use these deprecation mixins to
deprecate a function. This is because Sass doesn't allow you to include a mixin
in a function. For now, since we only have a few functions up for deprecation,
we can live with this and manually write warnings for those.
Add deprecation warnings for features that will be removed or changed in 5.0.0.

For more information on why most of these features are being removed, see:
#702
@tysongach tysongach merged commit fc9d1ee into v4-stable May 20, 2016
@tysongach tysongach deleted the tg-5-0-deprecations branch May 20, 2016 21:13
@avit
Copy link

avit commented Sep 21, 2017

In case anyone else got confused, the pull request description says:

$disable-bourbon-deprecation-warnings: true;

But the actual code has:

$output-bourbon-deprecation-warnings: true;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants