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

Correct handling of updater directory in sorbet #8247

Merged
merged 4 commits into from
Oct 23, 2023

Conversation

JamieMagee
Copy link
Contributor

The current configuration had 2 directories listed, . and updater. However, Sorbet only allows for a single directory. Additionally, we also had --ignore=updater. This configuration appeared to be accepted but hid some errors.

Also, a couple of points about todo.rbi:

  • Things should only ever be removed from todo.rbi, never added. Either:
    • They should be migrated to a hand-written .rbi in sorbet/rbi/shims
    • A spoom generated .rbi should be added in sorbet/rbi/gems
  • None of the Dependabot:: classes or modules should appear in todo.rbi
    • This is an indication that those files should be migrated to # typed: true

@JamieMagee JamieMagee requested a review from a team as a code owner October 20, 2023 21:45
@JamieMagee JamieMagee force-pushed the jamiemagee/sorbet-updater-config branch from 74b060d to d205c1e Compare October 20, 2023 21:53
@github-actions github-actions bot added L: php:composer Issues and code for Composer L: ruby:bundler RubyGems via bundler L: elixir:hex Elixir packages via hex L: java:gradle Maven packages via Gradle L: go:modules Golang modules L: github:actions GitHub Actions L: elm Elm packages L: git:submodules Git submodules L: terraform Terraform packages L: docker Docker containers L: rust:cargo Rust crates via cargo L: dotnet:nuget NuGet packages via nuget or dotnet L: java:maven Maven packages via Maven L: dart:pub Dart packages via pub L: javascript L: python L: swift Swift packages labels Oct 20, 2023
@JamieMagee
Copy link
Contributor Author

Also subsequently ran:

  • spoom bump --from=false --to=true
  • spoom bump --from=true --to=strict
  • spoom bump --from=strict --to=strong

To automatically update the type strength where possible, without any code changes.

Copy link
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

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

Nice catch, I guess I glossed over that ignore!

I wonder if we should put together a script/types that runs a series of idempotent typing related commands to make it easier to keep things in shipshape?

@jakecoffman jakecoffman merged commit 7de7855 into main Oct 23, 2023
97 checks passed
@jakecoffman jakecoffman deleted the jamiemagee/sorbet-updater-config branch October 23, 2023 12:52
@JamieMagee
Copy link
Contributor Author

I wonder if we should put together a script/types that runs a series of idempotent typing related commands to make it easier to keep things in shipshape?

I would start with making the sorbet PR check mandatory.

@JamieMagee JamieMagee added the sorbet 🍦 Relates to Sorbet types label Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dart:pub Dart packages via pub L: docker Docker containers L: dotnet:nuget NuGet packages via nuget or dotnet L: elixir:hex Elixir packages via hex L: elm Elm packages L: git:submodules Git submodules L: github:actions GitHub Actions L: go:modules Golang modules L: java:gradle Maven packages via Gradle L: java:maven Maven packages via Maven L: javascript L: php:composer Issues and code for Composer L: python L: ruby:bundler RubyGems via bundler L: rust:cargo Rust crates via cargo L: swift Swift packages L: terraform Terraform packages sorbet 🍦 Relates to Sorbet types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants