add types to DependencySnapshot #8986
Conversation
| @@ -170,7 +170,7 @@ def mark_job_as_processed(base_commit_sha) | |||
| span&.finish | |||
| end | |||
|
|
|||
| sig { params(dependencies: T::Array[T::Hash[Symbol, T.untyped]], dependency_files: T::Array[DependencyFile]).void } | |||
| sig { params(dependencies: T::Array[T::Hash[Symbol, T.untyped]], dependency_files: T::Array[String]).void } | |||
There was a problem hiding this comment.
I guess runtime validation isn't working for this part of the code because it's been wrong and I don't see any TypeErrors for it. It would happen for each job so we'd definitely notice.
There was a problem hiding this comment.
How come we're exposing this now, and can we verify that it's actually always taking a string?
There was a problem hiding this comment.
You can see in all the smoke tests, the data for the update_dependency_list endpoint's dependency_files is always an array of strings: https://github.com/dependabot/smoke-tests/blob/258269368c3c68116cf569bfed8bbafa5fcdc8e5/tests/smoke-bundler.yaml#L97
How come we're exposing this now
Now that we've got more and more files typed, things are falling into place and mistakes made earlier are getting fixed. If there was no runtime error then no problem I guess 🤷
|
|
||
| @dependencies = parse_files! | ||
| @dependencies = T.let(parse_files!, T::Array[Dependabot::Dependency]) |
There was a problem hiding this comment.
Kind of interesting that Sorbet doesn't infer the types for these methods that are defined in the same file.
I changed a few of the memoized methods into methods that calculate during initialization as the types work out much more easily this way.