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
Fix to deprecated S3DownloadStrategy #5137
Fix to deprecated S3DownloadStrategy #5137
Conversation
Remove references to removed require_aws_sdk method See commit: 599ecc9
@@ -204,12 +211,6 @@ def scp_source | |||
class DownloadStrategyDetector | |||
class << self | |||
module Compat | |||
def detect(url, using = nil) | |||
strategy = super | |||
require_aws_sdk if strategy == S3DownloadStrategy |
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.
Was the require_aws_sdk
method removed?
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.
Yeah, it looks like it got taken out during deprecation. I linked the commit in the PR description but here again for reference
599ecc9
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.
@amancevice My bad, could you re-add that method into this Compat
module instead?
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.
@amancevice Actually if this is working for you it's fine as-is.
Whoops, this wording isn't ideal, updating in #5138.
Hopefully the new messaging in #5138 is a bit clearer. In short, you're meant to move this code into your formula (or tap if you use I apologise that this is a relatively recent 180; ideally we'd have done this before accepting these download strategies but I think it's better for everyone if Homebrew isn't maintaining download strategies we don't actually use. |
Thanks again @amancevice! |
Remove references to removed require_aws_sdk method
See commit: 599ecc9b5ad7951b8ddc51490ebe93a976d43b29
I couldn't find any documentation on what exactly users are supposed to do with the warning:
Is there documentation on this?
brew style
with your changes locally?brew tests
with your changes locally?