-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Apply strict sorbet typings for python files #11908
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
base: main
Are you sure you want to change the base?
Conversation
python/lib/dependabot/python/file_updater/requirement_file_updater.rb
Outdated
Show resolved
Hide resolved
0cf594a
to
6b70b77
Compare
Do you see any problem with this PR? If not we can merge this?? |
000a0c9
to
951988a
Compare
def package_hashes_for(name:, version:, algorithm:) | ||
index_urls = @index_urls || [nil] | ||
index_urls = @index_urls&.any? ? @index_urls : [nil] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Tip: Instead of adding nil
to index_urls
before sending it, we've updated the place where we are using index_urls
to add nil
to the array if it's empty.
@@ -449,7 +457,7 @@ def deps_to_augment_hashes_for(updated_content, original_content) | |||
|
|||
sig { params(name: String, version: String, algorithm: String).returns(T::Array[String]) } | |||
def package_hashes_for(name:, version:, algorithm:) | |||
index_urls = @index_urls || [nil] | |||
index_urls = @index_urls&.any? ? @index_urls : [nil] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Tip: Instead of adding nil
to index_urls
before sending it, we've updated the place where we are using index_urls
to add nil
to the array if it's empty.
# If there are no credentials that replace the base, we need to | ||
# ensure that the base URL is included in the list of extra-index-urls. | ||
[nil, *urls] | ||
urls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Tip: Instead of adding nil
to index_urls
before sending it, we've updated the place where we are using index_urls
to add nil
to the array if it's empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can even omit this last urls
@@ -468,7 +468,7 @@ | |||
|
|||
context "when credentials do not replace base" do | |||
it "returns nil and authed urls for all credentials" do | |||
expect(instance.send(:pip_compile_index_urls)).to eq([nil, "authed_url"]) | |||
expect(instance.send(:pip_compile_index_urls)).to eq(["authed_url"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Tip: Instead of adding nil
to index_urls
before sending it, we've updated the place where we are using index_urls
to add nil
to the array if it's empty.
b83d61f
to
d1b6f01
Compare
|
||
old_req = T.must(T.must(dependency).previous_requirements) | ||
.find { |r| r[:file] == file.name } | ||
|
||
return file.content unless old_req | ||
return file.content if old_req == "==#{T.must(dependency).version}" | ||
content = file.content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to reassign content
?
To enhance the code robustness and make it more reliable by providing sorbet type fix as strict.
Added signature and made method body change to make it compatible with sorbet type strict.
Feature to add sorbet type as strict and make code more robust.
Anything you want to highlight for special attention from reviewers?
To check if the return type are correct.
How will you know you've accomplished your goal?
All the tests and linters pass successfully.
Checklist