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

Deprecate bundle plugin install --local-git= #7048

Merged

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Oct 10, 2023

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

Adding the --path param, I noticed --git and --local-git have identical implementations.

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

Keep --local-git as an undocumented alias for --git. Use the same actual implementation now. No functionality change.

Make sure the following tasks are checked

@ccutrer ccutrer force-pushed the deprecate-bundle-plugin-install-path branch from 6e065d4 to 63407e0 Compare October 10, 2023 14:02
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Looks great, just added a comment to add an explicit runtime deprecation.

I think this option is mostly discoverable through CLI help, and you already included a note about deprecation there, but we print a runtime message for other deprecated flags, so I think it does not hurt to also do it here.

bundler/lib/bundler/cli/plugin.rb Outdated Show resolved Hide resolved
@deivid-rodriguez
Copy link
Member

@ccutrer Sorry for my slow reviews, the plugin work you were doing is awesome. Whenever you're ready to continue, I am too!

@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 27, 2023

👍 I was on vacation for a week, then got Covid, so I'm a bit backlogged, but I promise to get back to it as soon as I can.

@deivid-rodriguez
Copy link
Member

Hope you enjoyed your vacation, and are feeling better now after Covid. Thanks so much for the work!

@ccutrer ccutrer force-pushed the deprecate-bundle-plugin-install-path branch 2 times, most recently from 51e6e5d to 190e962 Compare October 30, 2023 17:33
@deivid-rodriguez
Copy link
Member

Since the flag is deprecated in Bundler 2 and will make commands using it fail in Bundler 3, this is making some specs fail. If they are unrelated to the setting itself, they can be changed to use --git.

@ccutrer ccutrer force-pushed the deprecate-bundle-plugin-install-path branch from 190e962 to bf118b6 Compare October 30, 2023 20:04
@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 30, 2023

The failing specs are all ones I left in to ensure the deprecated behavior still works. I've add spec filters to them so they won't run at all under bundler 3.

@deivid-rodriguez
Copy link
Member

Sorry for the lack of feedback here.

By adding the filters, we'll lose the specs on Bundler 3, right? I think it's better to migrate the specs to use --git instead? We could limit usage of --local-git in specs to an explicit test at bundler/spec/other/major_deprecation_spec.rb. That's what we do for other deprecations.

@ccutrer
Copy link
Contributor Author

ccutrer commented Nov 8, 2023

Ahhh, yes, I could move the specs for deprecated features there. I didn't know it existed. The specs that have the filters are already duplicates of ones that use --git, so we won't lose coverage on Bundler 3.

@deivid-rodriguez
Copy link
Member

Are we testing that --git works with local git repos? I checked superficially and didn't find those specs.

@hsbt hsbt force-pushed the deprecate-bundle-plugin-install-path branch from bf118b6 to e2f97df Compare December 6, 2023 02:53
@deivid-rodriguez deivid-rodriguez force-pushed the deprecate-bundle-plugin-install-path branch 2 times, most recently from 149b929 to f605422 Compare March 13, 2024 16:10
@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Mar 13, 2024

I addressed what I think was left to do here.

It's the exact same implementation as --git
@deivid-rodriguez deivid-rodriguez force-pushed the deprecate-bundle-plugin-install-path branch from f605422 to 18eb241 Compare March 18, 2024 10:33
@deivid-rodriguez deivid-rodriguez merged commit 5c7246d into rubygems:master Mar 18, 2024
83 checks passed
deivid-rodriguez added a commit that referenced this pull request Mar 20, 2024
…-path

Deprecate `bundle plugin install --local-git=`

(cherry picked from commit 5c7246d)
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.

None yet

2 participants