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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make repo take precedence over sig/ in gem package #762

Conversation

pocke
Copy link
Member

@pocke pocke commented Aug 23, 2021

This PR changes loading priority between gem_repo and sig/ in gem package. Repo will take precedence over sig/.

Currently sig/ in gem package takes precedence over gem_repo. It means if there're RBSs in both of gem_repo and sig/, sig/ is used.
It is not a useful behavior. We can modify gem_repo's content easily, especially if we use rbs collection. But we need to fork the gem to modify sig/ directory in the gem package.
So, overriding RBS in sig/ with gem_repo is useful, but the opposite isn't.

I propose this PR to enable the overriding.


By the way, I'm wondering if EnvironmentLoader can receive the sources of RBS, which are gem_repo, sig/, or stdlibs.
I guess this PR is ok in most cases, but we need to specify the source to EnvLoader to make 100% sure that the loader loads RBS from expected places. I'll consider this problem 馃

@pocke pocke requested a review from soutaro August 23, 2021 15:55
@soutaro
Copy link
Member

soutaro commented Aug 26, 2021

I agree that adding an option to EnvironmentLoader is better way. Something like this?:

class EnvironmentLoader
  def add: (path: Pathname) -> void
         | (library: String, version: String?, ?from: Repository | :gem | nil) -> void
end

I'm still not very sure if making repository priority over gem by default is a good idea or not...

@pocke
Copy link
Member Author

pocke commented Sep 15, 2021

I'll close this issue. I and @soutaro discussed this PR and we agree with adding the from option.

@pocke pocke closed this Sep 15, 2021
@pocke pocke deleted the Make_repo_take_precedence_over_sig__in_gem_package branch September 15, 2021 12:53
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.

None yet

2 participants