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

set sidekiq redis direct dependency #493

Closed
wants to merge 1 commit into from

Conversation

HoneyryderChuck
Copy link
Contributor

@HoneyryderChuck HoneyryderChuck commented Jan 8, 2024

After rbs collection on a project with sidekiq, I'm getting " Cannot find type ::Redis" running steep. gems should be declaring their direct dependencies as well.

This is probably not the only "direct dependency" issue around, but it's enough to get the conversation started.

After `rbs collection` on a project with sidekiq, I'm getting " Cannot find type `::Redis`" running steep. gems should be declaring their direct dependencies as well.
@HoneyryderChuck HoneyryderChuck requested a review from pocke as a code owner January 8, 2024 11:04
@soutaro
Copy link
Member

soutaro commented Jan 8, 2024

gems should be declaring their direct dependencies as well.

Dependencies between gems are not managed by rbs-collection. It assumes Bundler takes care of them and load all of the required gems.
manifest.yaml is for default gems and non-gem standard libraries. Because the dependency to default gems are rarely declared in Gemfile or .gemspec.

The correct fix might be adding redis-client gem type definition in this repository.

Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

Submitting just to block someone accidentally merge this.

@HoneyryderChuck
Copy link
Contributor Author

@soutaro you're saying that sidekiq switching from "redis" to "redis-client" is the reason? ok, that was unfortunate then :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants