Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Better --force to --redownload transition #6702

Merged
7 commits merged into from Sep 24, 2018

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Sep 18, 2018

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

The problem was. that, while trying to run bundle install --force today, I was getting the error unknown switch --force error, and I was not sure what was causing it and what I needed to do to fix it. Also, running bundle install --help would show --force as a supported option, so that would make things even more confusing.

What was your diagnosis of the problem?

My diagnosis was that:

  • A lot of removal/renaming of options in bundler 2 are artificially linked to the forget_cli_options switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the forget_cli_options setting causing this rename to be in place.

  • We should show a helpful deprecation message instead of a hard error, so the transition is easier for users.

  • We should update the commands help to document newer instead of deprecated options.

  • The bundle update command has an equivalent --force option that should be renamed too for sanity.

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

My fix to the above problems is to:

  • Always keep the --force alias, independently of whether the forget_cli_options is set or not.
  • Show a deprecation message when --force is used in bundler 2, but still keep it working as before.
  • Update the commands help to use --redownload.
  • Make the equivalent changes to the bundle update

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.

@deivid-rodriguez deivid-rodriguez changed the title Better force to redownload transition Better --force to --redownload transition Sep 18, 2018
@indirect
Copy link
Member

hooray! this is perfect, and exactly the kind of thing I am hoping to accomplish with the 2.0 transition. 😄 I am extremely on board with the goals here, and I think this is a great change. thank you! 🙌

@bundlerbot r+

@bundlerbot
Copy link
Collaborator

📌 Commit c72a094 has been approved by indirect

@bundlerbot
Copy link
Collaborator

⌛ Testing commit c72a094 with merge 63fd4f3...

bundlerbot added a commit that referenced this pull request Sep 18, 2018
…r=indirect

Better `--force` to `--redownload` transition

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

The problem was. that, while trying to run `bundle install --force` today, I was getting the error `unknown switch --force` error, and I was not sure what was causing it and what I needed to do to fix it. Also, running `bundle install --help` would show `--force` as a supported option, so that would make things even more confusing.

### What was your diagnosis of the problem?

My diagnosis was that:

* A lot of removal/renaming of options in bundler 2 are artificially linked to the `forget_cli_options` switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the `forget_cli_options` setting causing this rename to be in place.

* We should show a helpful deprecation message instead of a hard error, so the transition is easier for users.

* We should update the commands help to document newer instead of deprecated options.

* The `bundle update` command has an equivalent `--force` option that should be renamed too for sanity.

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

My fix to the above problems is to:

* Always keep the `--force` alias, independently of whether the `forget_cli_options` is set or not.
* Show a deprecation message when `--force` is used in bundler 2, but still keep it working as before.
* Update the commands help to use `--redownload`.
* Make the equivalent changes to the `bundle update`

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.
ghost pushed a commit that referenced this pull request Sep 18, 2018
6702: Better `--force` to `--redownload` transition r=indirect a=deivid-rodriguez

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

The problem was. that, while trying to run `bundle install --force` today, I was getting the error `unknown switch --force` error, and I was not sure what was causing it and what I needed to do to fix it. Also, running `bundle install --help` would show `--force` as a supported option, so that would make things even more confusing.

### What was your diagnosis of the problem?

My diagnosis was that:

* A lot of removal/renaming of options in bundler 2 are artificially linked to the `forget_cli_options` switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the `forget_cli_options` setting causing this rename to be in place.

* We should show a helpful deprecation message instead of a hard error, so the transition is easier for users. 

* We should update the commands help to document newer instead of deprecated options.

* The `bundle update` command has an equivalent `--force` option that should be renamed too for sanity.

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

My fix to the above problems is to:

* Always keep the `--force` alias, independently of whether the `forget_cli_options` is set or not.
* Show a deprecation message when `--force` is used in bundler 2, but still keep it working as before.
* Update the commands help to use `--redownload`.
* Make the equivalent changes to the `bundle update`

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@bundlerbot
Copy link
Collaborator

💔 Test failed - status-travis

Instead of hard erroring, deprecate the `--force` flag on bundler 2 to
better teach users about the replacement.

Before:

$ bundle install --force
Unknown switches '--force'

After:

$ bundle install --force
[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`
Could not locate Gemfile # or whatever the expected behavior for --redownload is
@deivid-rodriguez deivid-rodriguez force-pushed the better_force_to_redownload_transition branch from c72a094 to 529998f Compare September 19, 2018 00:41
@ghost
Copy link

ghost commented Sep 19, 2018

Canceled

@deivid-rodriguez
Copy link
Member Author

@bundlerbot r=indirect

ghost pushed a commit that referenced this pull request Sep 24, 2018
6702: Better `--force` to `--redownload` transition r=indirect a=deivid-rodriguez

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

The problem was. that, while trying to run `bundle install --force` today, I was getting the error `unknown switch --force` error, and I was not sure what was causing it and what I needed to do to fix it. Also, running `bundle install --help` would show `--force` as a supported option, so that would make things even more confusing.

### What was your diagnosis of the problem?

My diagnosis was that:

* A lot of removal/renaming of options in bundler 2 are artificially linked to the `forget_cli_options` switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the `forget_cli_options` setting causing this rename to be in place.

* We should show a helpful deprecation message instead of a hard error, so the transition is easier for users. 

* We should update the commands help to document newer instead of deprecated options.

* The `bundle update` command has an equivalent `--force` option that should be renamed too for sanity.

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

My fix to the above problems is to:

* Always keep the `--force` alias, independently of whether the `forget_cli_options` is set or not.
* Show a deprecation message when `--force` is used in bundler 2, but still keep it working as before.
* Update the commands help to use `--redownload`.
* Make the equivalent changes to the `bundle update`

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@ghost
Copy link

ghost commented Sep 24, 2018

Build succeeded

@ghost ghost merged commit 2a2f425 into master Sep 24, 2018
@deivid-rodriguez deivid-rodriguez deleted the better_force_to_redownload_transition branch September 24, 2018 11:29
ghost pushed a commit that referenced this pull request Sep 25, 2018
6707: Run more assertions in more cases  r=deivid-rodriguez a=deivid-rodriguez

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

I noticed a couple of places where assertions were being excluded and they shouldn't:

* One was introduced by me in #6702, where the specs added (and some already present) started being tested only on bundler 2.x.
* The other one was introduced in f7414bc, where one assertion would be run only if a certain env variable was not set. I think it was because of a TravisCI environmental issue that now seems fixed.

### What was your diagnosis of the problem?

My diagnosis was that none of these exclusions are necessary.

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

My fix is to restore the excluded assertions to all environments and branches.

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

I chose this fix because it seems best to avoid future problems.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@deivid-rodriguez deivid-rodriguez added this to the 1.17.0 milestone Oct 6, 2018
@colby-swandale
Copy link
Member

Note that for 1.17.0, the documentation for --force cannot be removed. Either both options needs to be specified in the docs or just use --force for the time being until the next major release.

@deivid-rodriguez
Copy link
Member Author

You're totally right. I think we should document --redownload, and say --force is an alias?

colby-swandale pushed a commit that referenced this pull request Oct 9, 2018
6702: Better `--force` to `--redownload` transition r=indirect a=deivid-rodriguez

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

The problem was. that, while trying to run `bundle install --force` today, I was getting the error `unknown switch --force` error, and I was not sure what was causing it and what I needed to do to fix it. Also, running `bundle install --help` would show `--force` as a supported option, so that would make things even more confusing.

### What was your diagnosis of the problem?

My diagnosis was that:

* A lot of removal/renaming of options in bundler 2 are artificially linked to the `forget_cli_options` switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the `forget_cli_options` setting causing this rename to be in place.

* We should show a helpful deprecation message instead of a hard error, so the transition is easier for users. 

* We should update the commands help to document newer instead of deprecated options.

* The `bundle update` command has an equivalent `--force` option that should be renamed too for sanity.

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

My fix to the above problems is to:

* Always keep the `--force` alias, independently of whether the `forget_cli_options` is set or not.
* Show a deprecation message when `--force` is used in bundler 2, but still keep it working as before.
* Update the commands help to use `--redownload`.
* Make the equivalent changes to the `bundle update`

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
(cherry picked from commit 16ebc81)
colby-swandale pushed a commit that referenced this pull request Oct 9, 2018
6707: Run more assertions in more cases  r=deivid-rodriguez a=deivid-rodriguez

I noticed a couple of places where assertions were being excluded and they shouldn't:

* One was introduced by me in #6702, where the specs added (and some already present) started being tested only on bundler 2.x.
* The other one was introduced in f7414bc, where one assertion would be run only if a certain env variable was not set. I think it was because of a TravisCI environmental issue that now seems fixed.

My diagnosis was that none of these exclusions are necessary.

My fix is to restore the excluded assertions to all environments and branches.

I chose this fix because it seems best to avoid future problems.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
(cherry picked from commit 3d9e616)
@deivid-rodriguez
Copy link
Member Author

@colby-swandale Thoughts about doing something like this, and removing the "deprecated" word when backporting it?

@colby-swandale
Copy link
Member

I like the idea but i would prefer saying that --redownload is the alias for 1.X releases.

@deivid-rodriguez
Copy link
Member Author

Sounds good to me, let me cook a PR!

@deivid-rodriguez
Copy link
Member Author

Actually, after another thought I don't think we should document deprecated options, so it's better if I target the PR directly to 1.17-stable. Sounds good?

@colby-swandale
Copy link
Member

sounds great!

deivid-rodriguez pushed a commit that referenced this pull request Oct 11, 2018
6702: Better `--force` to `--redownload` transition r=indirect a=deivid-rodriguez

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

The problem was. that, while trying to run `bundle install --force` today, I was getting the error `unknown switch --force` error, and I was not sure what was causing it and what I needed to do to fix it. Also, running `bundle install --help` would show `--force` as a supported option, so that would make things even more confusing.

### What was your diagnosis of the problem?

My diagnosis was that:

* A lot of removal/renaming of options in bundler 2 are artificially linked to the `forget_cli_options` switch. To me, those changes even though they are related (stopping rememebering cli options prompted us to actually remove some unnecessary ones), they are independent, so it's not straighforward to find out that it's the `forget_cli_options` setting causing this rename to be in place.

* We should show a helpful deprecation message instead of a hard error, so the transition is easier for users.

* We should update the commands help to document newer instead of deprecated options.

* The `bundle update` command has an equivalent `--force` option that should be renamed too for sanity.

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

My fix to the above problems is to:

* Always keep the `--force` alias, independently of whether the `forget_cli_options` is set or not.
* Show a deprecation message when `--force` is used in bundler 2, but still keep it working as before.
* Update the commands help to use `--redownload`.
* Make the equivalent changes to the `bundle update`

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

I chose this fix because it seems like the more user friendly way going forward in my opinion.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
(cherry picked from commit 16ebc81)
@deivid-rodriguez
Copy link
Member Author

Here you go! 👉 #6734

colby-swandale added a commit that referenced this pull request Oct 25, 2018
* 1-17-stable: (38 commits)
  Version 1.17.0 with changelog
  Merge #6754
  Version 1.17.0.pre.2 with changelog
  fix failing bundle remove specs
  Still document the `--force` option
  scope specs testing bundler 2 deprecations to bundler 1 only
  Merge #6718
  Merge #6707
  Merge #6702
  Merge #6316
  Auto merge of #6447 - agrim123:agr-update-error-message, r=segiddins
  Auto merge of #6513 - agrim123:agr-bundler-remove, r=indirect
  Auto merge of #6318 - jhawthorn:fix_incorrect_test_in_requires_sudo, r=segiddins
  Auto merge of #6450 - bundler:segiddins/bundle-binstubs-all, r=colby-swandale
  Auto merge of #6024 - gwerbin:custom-user-bundle-dir, r=colby-swandale
  Version 1.17.0.pre.1 with changelog
  Auto merge of #5964 - bundler:colby/deprecate-viz-command, r=segiddins
  Auto merge of #5986 - bundler:seg-jobs-count, r=indirect
  Auto merge of #5995 - bundler:seg-gvp-major, r=indirect
  Auto merge of #5803 - igorbozato:path_relative_to_pwd, r=indirect
  ...
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants