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

Nev fixes after steep review #32

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Conversation

nevinera
Copy link
Owner

@nevinera nevinera commented Nov 20, 2023

I got a review on #28 after it merged that had some good points, and I wanted to go ahead and get them taken care of.

  • The :test config in the Steepfile would be for a 'test/' directory, which we don't have. In general, RBS doesn't do much for rspec tests, since they are constructed as massive piles of anonymous metaprogrammed modules and classes (which don't have any type information).
  • Use the ?Foo syntax to specify the optional parameter to fetch, instead of listing multiple signatures (it's equivalent, but the former is clearer and better)
  • Stop managing the bundler cache and gem installation ourselves in the rspec github workflows

(I also tried to use groups to manage the installation of the steep gem instead of an environment variable but.. failed. See here for details)

Copy link
Collaborator

@rbellido-ut rbellido-ut left a comment

Choose a reason for hiding this comment

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

👍🏽

@nevinera nevinera merged commit 1da1932 into main Nov 22, 2023
7 checks passed
@nevinera nevinera deleted the nev--fixes-after-steep-review branch November 22, 2023 23:05
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