Skip to content

feat: Add support for manually specifiying gems for cross-repo. #149

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

Merged
merged 3 commits into from
Nov 4, 2022

Conversation

varungandhi-src
Copy link
Contributor

@varungandhi-src varungandhi-src commented Nov 2, 2022

Motivation

This would help customers specify cross-repo mappings in an easy way
even when our inference fails.

Test plan

  • Add unit test for JSON parsing
  • Manually test paths with some files. Main thing to check here is both relative and absolute paths in the JSON are accepted.
  • Add test for the stdlib version being used. We aren't using specific versions, we can add that later.
  • Add support for multi-gem tests (this is a TODO in the test runner).
  • Add multi-gem tests where the JSON file is passed from outside. We need to add tests for both relative and absolute paths.
    • Relative path - snapshot test
    • Absolute path - unit test

@varungandhi-src
Copy link
Contributor Author

Interesting; if I put a print statement inside enterFile, that ends up using GitHub links for RBI paths like

https://github.com/sorbet/sorbet/tree/master/rbi/stdlib/yaml.rbi

@varungandhi-src
Copy link
Contributor Author

OK, so this does seem to be working OK for basic file lists, but the fallback to the --gem-metadata value is leading to misassignment of package name/version for stdlib stuff like Kernel#puts etc. We should fix that.

@varungandhi-src varungandhi-src force-pushed the vg/manual-cross-repo branch 4 times, most recently from 3fa112c to 65d17ac Compare November 4, 2022 08:04
@@ -35,7 +35,6 @@ def m2
end

sig { params(C, T::Boolean).returns(T::Boolean) }
# ^ reference [..] `T.untyped`#
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Erm, why is this 'C' and not 'C1' and why are parameter types missing?

@varungandhi-src varungandhi-src merged commit b86fe5f into scip-ruby/master Nov 4, 2022
@varungandhi-src varungandhi-src deleted the vg/manual-cross-repo branch November 4, 2022 12:43
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.

1 participant