Added sorbet typecheck under python module on 5 files #11261
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
randhircs
changed the title
randhircs
force-pushed
the
randhircs/dependabot-core-sorbet-python01
branch
from
0032c61 to
601f7de
Compare
markhallen
previously approved these changes
Jan 10, 2025
abdulapopoola
requested changes
Jan 10, 2025
There was a problem hiding this comment.
You have to change the typed:true to typed:strict; see this example
|
Sure, I change it. |
randhircs
force-pushed
the
randhircs/dependabot-core-sorbet-python01
branch
from
1b814d7 to
6799716
Compare
randhircs
force-pushed
the
randhircs/dependabot-core-sorbet-python01
branch
from
6799716 to
bf7b0ec
Compare
randhircs
force-pushed
the
randhircs/dependabot-core-sorbet-python01
branch
from
aca58f5 to
955f98a
Compare
| @@ -198,14 +198,15 @@ def setup_python_environment | |||
| nil | |||
| end | |||
|
|
|||
| sig { params(package_manager: String, version: String).void } | |||
| sig { params(package_manager: String, version: String).returns(T.nilable(T::Boolean)) } | |||
There was a problem hiding this comment.
why is this a nilable boolean?
There was a problem hiding this comment.
Yes, I also think all conditions are handled. It shouldn't be nillable.
There was a problem hiding this comment.
Yeah, I corrected it now, thanks to both of you.
kbukum1
reviewed
Jan 14, 2025
| @@ -32,7 +33,7 @@ class FileParser < Dependabot::FileParsers::Base | |||
| pipfile: "dev-packages", | |||
| lockfile: "develop" | |||
| } | |||
| ].freeze | |||
| ].freeze, T::Array[T::Hash[T.untyped, T.untyped]]) | |||
There was a problem hiding this comment.
Suggested Change: Instead of using T::Hash[T.untyped, T.untyped], use T::Hash[Symbol, String] (or another more specific type) to improve type safety and clarity.
Reasoning:
T.untypedallows any type, which weakens Sorbet’s ability to provide meaningful type checks.- By specifying
Symbolas the key type andStringas the value type (or another suitable type if necessary), you:- Make the code's intent clearer.
- Enable Sorbet to catch potential mismatches early.
- Provide better documentation for others reading or maintaining the code.
randhircs
force-pushed
the
randhircs/dependabot-core-sorbet-python01
branch
from
90c46ea to
64be557
Compare
randhircs
dismissed
kbukum1’s stale review
Since all the issues are fixed and due to time constraint , dismissing the review.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Requirement is to add Sorbet TypeCheck under Python module, Selected files are updated with TypeCheck.
Anything you want to highlight for special attention from reviewers?
TypeCheck are having correct data type params and return data type.
How will you know you've accomplished your goal?
Checklist