-
Notifications
You must be signed in to change notification settings - Fork 22k
Add an explicit dependency on json gem #55847
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
Conversation
Why does it need to be explicit? |
Yes, |
Hum, I see. I need to think a bit about this. |
json is hardly the only gem where this would apply. These are also used by rails in some place and have a gem available: The gem is explicitly marked as |
Me too, but it's my understanding that encouraging, if not forcing, that is an intended goal of stdgems work |
Well, if at one point json moves to be bundled like many other gems already have then there is really no way around doing something like that. But up to the point where that actually happens, I am heavily -1 on this. Feels like JavaScript. I already heavily dislike the move to make many of the commonly used gems bundled and I know I am not the only one with that sentiment. Fine, move webrick to bundled, I see the point of that, but who thought it was a good idea to do the same with base64 that basically just wraps a single method call?? /rant over/ |
Yeah, that's a lot of dependencies. On one end, they already are so it just makes them explicit, but that "looks" weird. But I also like the idea that by listing these dependencies we make it more likely that users will upgrade them. So I'm warming up to the idea of listing |
Signed-off-by: Takuya Noguchi <takninnovationresearch@gmail.com>
48276e5
to
4646cc4
Compare
I force pushed the PR to update the Gemfile.lock. |
I am not on listing every stdgem at all.
Per @Earlopain's list, we will have four gems as follows:
This is why I think we should only add the json gem as a dependency to The used commands: Full content of
|
Motivation / Background
ActiveSupport depends on
json
gem, which has been This pull request makes a small update to theactivesupport.gemspec
file, adding a new dependency to the gem specification.Fixes #55853
Detail
This pull request adds
json
as a dependency in theGem::Specification
foractivesupport
, ensuring that the gem explicitly requires thejson
library.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]