Skip to content

Add more dependencies of standard libraries#29

Merged
mame merged 1 commit intoruby:masterfrom
pocke:Add_more_dependencies_of_standard_libraries
Jan 21, 2021
Merged

Add more dependencies of standard libraries#29
mame merged 1 commit intoruby:masterfrom
pocke:Add_more_dependencies_of_standard_libraries

Conversation

@pocke
Copy link
Member

@pocke pocke commented Jan 11, 2021

This pull request adds more fixed dependencies to standard libraries.

Problem

Currently typeprof command fails if a Ruby code has require 'logger'. For example:

# test.rb

require 'logger'
$ typeprof test.rb
# Analysis Error
A constant `MonitorMixin' is used but not defined in RBS

I know this problem because of a Qiita article. https://qiita.com/kure/items/6f93429978466d2860c7#typeprof%E3%81%A7%E3%81%AE%E8%A9%A6%E3%81%BF

Cause

Logger actually depends on MonitorMixin module that is defined by monitor library. But typeprof doesn't know the dependency, so it doesn't load monitor library.

Solution

Add monitor library to the loader if logger is required. This patch also adds other dependencies.
The list came from here: https://github.com/ruby/rbs/blob/4e5ee848b318c6c9cd4131a825f0b794017acb3a/Rakefile#L35-L54

I know it is an ad-hoc approach, but I think we need the approach currently. Because RBS doesn't have any feature to manage dependency. And actually the users couldn't use typeprof by this problem.

@pocke
Copy link
Member Author

pocke commented Jan 11, 2021

The CI is failed wiht the minitest dependency problem on testbed/goodcheck-Gemfile.lock.

@mame mame merged commit 2f09eb5 into ruby:master Jan 21, 2021
@mame
Copy link
Member

mame commented Jan 21, 2021

Sorry for my late action. I've updated minitest to 5.14.3, so I believe that the test will work. Thank you very much!

@pocke pocke deleted the Add_more_dependencies_of_standard_libraries branch January 21, 2021 04:06
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.

2 participants