-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
language/*: enable typed: strict
#16971
Conversation
Also add comment to odeprecate in future. In homebrew/core, only usage is in deprecated/disabled formulae. Signed-off-by: Michael Cho <michael@michaelcho.dev>
Signed-off-by: Michael Cho <michael@michaelcho.dev>
Signed-off-by: Michael Cho <michael@michaelcho.dev>
Signed-off-by: Michael Cho <michael@michaelcho.dev>
def self.stage_deps(resources, target) | ||
# odeprecated "Language::Go::stage_deps", "or request upstream to migrate to Go modules" |
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.
Is it worth saying this is a future intended deprecation (in a comment above) otherwise it might be confusing for people combing through deprecations.
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.
@issyl0 This is how we normally handle "deprecate this next major/minor release" deprecations.
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.
Ah, thanks for the clarification!
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.
Maybe we can reword this somehow? Otherwise we end up with
use or request upstream to migrate to Go modules 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.
I was originally either going to leave it empty or thinking of just Go modules
(i.e. use Go modules instead
)
I partly mixed in another related message from:
brew/Library/Homebrew/rubocops/text.rb
Line 117 in 9ae5161
problem "`go_resource`s are deprecated. Please ask upstream to implement Go vendoring" |
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 can do any extra updates to deprecation message in a separate PR and also add comment for go_resource
s. May just go with something like:
# odeprecated "Language::Go::stage_deps", "or request upstream to migrate to Go modules" | |
# odeprecated "Language::Go::stage_deps", "Go modules" |
In case of existing message, as I understand, vendoring would also refer to older solution with vendor directory and GOPATH, which we are also trying to get rid of.
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 guess another option is to recommend: Language::Go::stage_deps --> resources.each { |r| (target/r.name).install r}
and go_resource --> resource
. Though, this means the formula is still using an old approach.
I have used these to get rid of all usage of go_resources
in Homebrew/core.
def self.stage_deps(resources, target) | ||
# odeprecated "Language::Go::stage_deps", "or request upstream to migrate to Go modules" |
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.
@issyl0 This is how we normally handle "deprecate this next major/minor release" deprecations.
Looks good, thanks again @cho-m! |
Co-authored-by: Markus Reiter <me@reitermark.us>
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?