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

Git Sources shorthands: Deprecation note #513

Closed
wants to merge 2 commits into from
Closed

Git Sources shorthands: Deprecation note #513

wants to merge 2 commits into from

Conversation

olleolleolle
Copy link
Member

@olleolleolle olleolleolle commented Apr 9, 2020

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

The problem was that the deprecated status of the built-in shorthands was not documented.

Fixes #503

What was your diagnosis of the problem?

My diagnosis was that @duckinator was right about the way to deal with the shorthands in the documentation.

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

My fix was to document the deprecation (v2.0/ directory) with a NB marker.

Update: Added "what to do" as a suggestion.

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

I chose this fix because I didn't know about any special tricks to show "All This Section Is Deprecated".

@simi
Copy link
Member

simi commented Apr 30, 2020

From another point of view, if we would like to not promote this, wouldn't be less confusing to just remove this example?

@olleolleolle
Copy link
Member Author

olleolleolle commented Apr 30, 2020

I think it's important that users can look in the docs, learn what it is they see in some project's Gemfile. Even if that feature will be going away in the future.

That said, perhaps this is a case for switching the order of the content around. "Custom git sources" could be placed first, and then, as an afterthought, the Shorthand built-in git sources.

@simi
Copy link
Member

simi commented Apr 30, 2020

sounds good to me

@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Apr 30, 2020

I believe we need to fix something here in bundler, because using these shortcuts doesn't currently print any deprecation warnings in bundler 2, but they are broken in bundler 3.

So we need to either fix bundler to print deprecations for them, or to get them back to working and not deprecate them at all.

@deivid-rodriguez
Copy link
Member

Ok, I'm digging through history now and we decided to delay the deprecation of these to bundler 3: rubygems/bundler#7000.

The rationale if I recall correctly was that we were also in the middle of switching these sources to use https instead of git under the hood by default, so deprecating everything at the same time was a bit of a mess.

Well, and honestly, I kind of like these shortcuts, I'm not sure about the reason for deprecating them but I assume it was a good reason, so I didn't dig deeper.

Anyways, there's a bug in bundler since in bundler 3, these sources are broken like this:

 $ bundle

[!] There was an error parsing `Gemfile`: You passed :github as an option for gem 'rails', but it is invalid. Valid options are: group, groups, git, path, glob, name, branch, ref, tag, require, submodules, platform, platforms, type, source, install_if, gemfile. You may be able to resolve this by upgrading Bundler to the newest version.. Bundler cannot continue.

 #  from /home/deivid/Code/playground/bundler/shortcuts/Gemfile:5
 #  -------------------------------------------
 #  
 >  gem "rails", github: "rails/rails"
 #  -------------------------------------------

where the expectation is to only warn.

Regarding documenting this at this stage, I don't have a strong opinion.

@olleolleolle
Copy link
Member Author

Thanks for all the review effort! I will close this and not change anything at this point, we can return to it when Bundler 3 is coming around, and then we'll have all these words & thoughts to look at, if we ever need them. Ciao!

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.

Installing gems from gists is underdocumented.
3 participants