Turn on deprecations by default #6965

deivid-rodriguez · 2019-02-14T13:38:42Z

What was the end-user problem that led to this PR?

The problem was we've had deprecations in place for a long time, but we've never displayed them to users by default.

What was your diagnosis of the problem?

My diagnosis was that the current strategy doesn't work because:

Printing deprecations as an optin feature almost never get enabled, so most users don't know about the stuff we'll be deprecating in the future.
Printing deprecations in "deprecation releases" doesn't work well either, because it's unclear how long the deprecation release should last and thus how long we need to hold the final release (that will inhibit installation of the deprecation release).

What is your fix for the problem, implemented in this PR?

My fix is to remove the concept of deprecation releases, and to add a feature flag for printing major deprecations that it's enabled by default.

As a extra related change, I also reworded the deprecation messages, because I find the current message "[DEPRECATED FOR 2.0] " a bit confusing because it's unclear what the version printed is referring to (deprecation horizon? current running version?), so I changed it to just "[DEPRECATED] ".

Why did you choose this fix out of the possible options?

I chose this fix because it (once released) finally makes it so that users will know about our deprecations.

deivid-rodriguez · 2019-02-14T17:06:19Z

@indirect @segiddins You ok with this path forward? I know this was not the original plan, but I read #6586 (comment) and #4855 (comment) and I think we all agree about this now?

colby-swandale · 2019-02-18T21:17:24Z

It's normal for users to have many versions of Bundler installed, so i find the deprecation message with the version number pretty handy to know which major version of Bundler has deprecated/removed what.

colby-swandale · 2019-02-18T21:21:06Z

lib/bundler/feature_flag.rb

@@ -44,6 +44,7 @@ def self.settings_method(name, key, &default)
    settings_flag(:init_gems_rb) { bundler_2_mode? }
    settings_flag(:list_command) { bundler_2_mode? }
    settings_flag(:lockfile_uses_separate_rubygems_sources) { bundler_2_mode? }
+    settings_flag(:major_deprecations) { bundler_2_mode? }


IMO this should not be a feature flag as this is not enabling any particular Bundler feature and would also never be removed either. Can we just set the config option to have a default value instead?

That makes sense to me, I'm going to change it as you suggest 👍.

deivid-rodriguez · 2019-02-18T22:50:04Z

It's normal for users to have many versions of Bundler installed, so i find the deprecation message with the version number pretty handy to know which major version of Bundler has deprecated/removed what.

It was confusing for me because the first time I saw it I didn't know whether it meant the deprecation horizon (when the feature will be removed), the currently running version, or the version that removed the feature.

Anyways, I don't really find the version that deprecated a feature very useful for users, but I can restore it if others feel differently. If I do restore it, should I at least remove the trailing ".0"? If we deprecate a feature on bundler 2.1, won't that be incorrect?

deivid-rodriguez · 2019-02-18T22:50:27Z

By the way, thanks so much for having a look at this @colby-swandale !

indirect

I agree that just turning it on seems reasonable, and the "FOR 2.0" part was not clear.

As for the setting, maybe we can make it always default to on, with a setting named silence_deprecations that you can set to true if you want to hide deprecations?

deivid-rodriguez · 2019-02-19T13:20:24Z

Ok, so it turns out the feature flag is needed so that we can keep the 1.x specs passing. The way we've been using feature flags is so that we can virtually maintain a single branch, and the only difference between branches is the running bundler version (which triggers a different set of specs).

Enabling deprecations inconditionally made the deprecations be also enabled in bundler 1, and that causes tests to fail. Doing this revealed a few problems with our tests and deprecations that we should address though, so tentatively removing the feature flag was a good thing 😃.

In particular, our tests are using deprecated behaviour. If we don't use the new behavior ourselves, how can we encourage users to do it? 😅. I'll be addressing this on a separate PR, so that our tests always use preferred non deprecated behaviour unless we are testing the deprecations themselves.

Creating a PR to fix the above also surfaced a few problems with some of the deprecations themselves. I'll create issues about these problems that I found as well.

As per what to do with this PR. I see several options:

Restore the feature flag, at least for the time being.
Remove the 1.x build, and get ready to maintain several branches. Our feature flag strategy didn't fully work after all, and we're maintaining several branches anyways.
Hold the PR until I address the above issues.

deivid-rodriguez · 2019-02-19T13:30:53Z

Oh, another idea: explicitly toggle off deprecations via environment for the 1.x build.

indirect · 2019-02-19T23:55:44Z

Adding those updates to the tests sounds great! Also turning off deprecations for the versions before the deprecations are enabled sounds good to me. :)

deivid-rodriguez · 2019-02-20T00:21:35Z

Also turning off deprecations for the versions before the deprecations are enabled sounds good to me.

Ok, so that means turning them off via environment as suggested in #6965 (comment), right? Just double checking because... well, that is essentially what the feature flag was doing, so maybe I can just restore that.

As for the setting, maybe we can make it always default to on, with a setting named silence_deprecations that you can set to true if you want to hide deprecations?

Regarding this, it sounds like a good idea because it's great naming, but it implies introducing yet another setting to do the same thing. Would we remove (or deprecate 😱) the current setting, or just add it as a "nice redundancy"?

indirect · 2019-02-20T01:10:56Z

that means turning them off via environment as suggested in #6965 (comment), right?

Yup, that seems super reasonable to me. 👍

Would we remove (or deprecate 😱) the current setting, or just add it as a "nice redundancy"?

I would be okay with either renaming the setting or adding it as the "user-facing" name for this setting, to provide a better user experience.

deivid-rodriguez · 2019-02-21T15:11:05Z

I changed the name of the setting to silence_deprecations, and regarding the failing 1.x specs, it was due to a bug in deprecations that I fixed 👉 cf7fa53.

deivid-rodriguez · 2019-02-21T15:11:38Z

@indirect May I ask for a final look here?

Including the version is confusing, in my opinion, because it's unclear whether it refers to the future version of removal, or to the current running version.

Leave it as a setting with default true value.

Deprecations enabled or not, we only want to print deprecations for the running major version, never for future versions.

Since deprecations are enabled by default.

indirect · 2019-02-22T07:37:26Z

Awesome! :)

@bundlerbot r+

6965: Turn on deprecations by default r=indirect a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was we've had deprecations in place for a long time, but we've never displayed them to users by default. ### What was your diagnosis of the problem? My diagnosis was that the current strategy doesn't work because: * Printing deprecations as an optin feature almost never get enabled, so most users don't know about the stuff we'll be deprecating in the future. * Printing deprecations in "deprecation releases" doesn't work well either, because it's unclear how long the deprecation release should last and thus how long we need to hold the final release (that will inhibit installation of the deprecation release). ### What is your fix for the problem, implemented in this PR? My fix is to remove the concept of deprecation releases, and to add a feature flag for printing major deprecations that it's enabled by default. As a extra related change, I also reworded the deprecation messages, because I find the current message "[DEPRECATED FOR 2.0] <Message about the deprecation>" a bit confusing because it's unclear what the version printed is referring to (deprecation horizon? current running version?), so I changed it to just "[DEPRECATED] <Message about the deprecation>". ### Why did you choose this fix out of the possible options? I chose this fix because it (once released) finally makes it so that users will know about our deprecations. Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

ghost · 2019-02-22T08:09:35Z

Build succeeded

continuous-integration/travis-ci/push

deivid-rodriguez mentioned this pull request Feb 14, 2019

Restore 2.0 deprecations #6933

Closed

colby-swandale reviewed Feb 18, 2019

View reviewed changes

indirect approved these changes Feb 19, 2019

View reviewed changes

deivid-rodriguez force-pushed the deprecation_changes branch from af227e3 to 112ac3f Compare February 19, 2019 10:38

deivid-rodriguez force-pushed the deprecation_changes branch 4 times, most recently from 56384b8 to 227007f Compare February 21, 2019 14:21

deivid-rodriguez added 8 commits February 21, 2019 16:32

Change some specs checking the wrong stream

2610e6c

Reword deprecation messages

149f1ce

Including the version is confusing, in my opinion, because it's unclear whether it refers to the future version of removal, or to the current running version.

Remove deprecation releases

f626de2

Turn on deprecations by default

a6f593f

Remove major_deprecations feature flag

76c411d

Leave it as a setting with default true value.

Fix deprecations bug

82d4176

Deprecations enabled or not, we only want to print deprecations for the running major version, never for future versions.

Remove now redundant configs

85f194d

Since deprecations are enabled by default.

Rename major_deprecations to silence_deprecations

fda9db6

deivid-rodriguez mentioned this pull request Feb 21, 2019

Reenable deprecation specs (and make up a plan) #6975

Closed

5 tasks

deivid-rodriguez force-pushed the deprecation_changes branch from 227007f to fda9db6 Compare February 21, 2019 17:47

ghost merged commit fda9db6 into master Feb 22, 2019

ghost deleted the deprecation_changes branch February 22, 2019 08:09

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn on deprecations by default #6965

Turn on deprecations by default #6965

deivid-rodriguez commented Feb 14, 2019

deivid-rodriguez commented Feb 14, 2019

colby-swandale commented Feb 18, 2019

colby-swandale Feb 18, 2019

deivid-rodriguez Feb 18, 2019

deivid-rodriguez commented Feb 18, 2019 •

edited

deivid-rodriguez commented Feb 18, 2019

indirect left a comment

deivid-rodriguez commented Feb 19, 2019

deivid-rodriguez commented Feb 19, 2019

indirect commented Feb 19, 2019

deivid-rodriguez commented Feb 20, 2019

indirect commented Feb 20, 2019 •

edited

deivid-rodriguez commented Feb 21, 2019

deivid-rodriguez commented Feb 21, 2019

indirect commented Feb 22, 2019

ghost commented Feb 22, 2019

Turn on deprecations by default #6965

Turn on deprecations by default #6965

Conversation

deivid-rodriguez commented Feb 14, 2019

What was the end-user problem that led to this PR?

What was your diagnosis of the problem?

What is your fix for the problem, implemented in this PR?

Why did you choose this fix out of the possible options?

deivid-rodriguez commented Feb 14, 2019

colby-swandale commented Feb 18, 2019

colby-swandale Feb 18, 2019

Choose a reason for hiding this comment

deivid-rodriguez Feb 18, 2019

Choose a reason for hiding this comment

deivid-rodriguez commented Feb 18, 2019 • edited

deivid-rodriguez commented Feb 18, 2019

indirect left a comment

Choose a reason for hiding this comment

deivid-rodriguez commented Feb 19, 2019

deivid-rodriguez commented Feb 19, 2019

indirect commented Feb 19, 2019

deivid-rodriguez commented Feb 20, 2019

indirect commented Feb 20, 2019 • edited

deivid-rodriguez commented Feb 21, 2019

deivid-rodriguez commented Feb 21, 2019

indirect commented Feb 22, 2019

ghost commented Feb 22, 2019

Build succeeded

deivid-rodriguez commented Feb 18, 2019 •

edited

indirect commented Feb 20, 2019 •

edited