Revert "chore: bump cloud.google.com/go/logging from 1.13.2 to 1.15.0"#2130
Revert "chore: bump cloud.google.com/go/logging from 1.13.2 to 1.15.0"#2130
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request downgrades several Google Cloud dependencies, including logging, IAM, and longrunning, and updates the vendored source code. The review feedback identifies several regressions introduced by these changes, specifically the use of the deprecated 'io/ioutil' package and instances of variable shadowing in generated files. These issues should be addressed to maintain code quality and adhere to modern Go standards.
| package internal | ||
|
|
||
| import ( | ||
| "io/ioutil" |
There was a problem hiding this comment.
The 'io/ioutil' package is deprecated since Go 1.16. This import should be removed if the usage of 'ioutil.ReadFile' is replaced with 'os.ReadFile'.
References
- Linting is done with golangci-lint, which typically flags the use of deprecated packages like 'io/ioutil'. (link)
- The 'io/ioutil' package is deprecated. Use the 'os' or 'io' packages instead.
| func (g *defaultResourceGetter) ReadAll(path string) string { | ||
| bytes, err := os.ReadFile(path) | ||
| bytes, err := ioutil.ReadFile(path) | ||
| if err != nil { |
There was a problem hiding this comment.
The 'io/ioutil' package is deprecated since Go 1.16. It is recommended to use 'os.ReadFile' instead, which is already available in the 'os' package imported in this file.
bytes, err := os.ReadFile(path)References
- Linting is done with golangci-lint, which flags the use of deprecated functions like 'ioutil.ReadFile'. (link)
- The 'io/ioutil' package is deprecated. Use 'os.ReadFile' instead.
| if x, ok := x.GetPayload().(*LogEntry_ProtoPayload); ok { | ||
| return x.ProtoPayload | ||
| } |
There was a problem hiding this comment.
Shadowing the receiver variable 'x' with the result of a type assertion is confusing and can lead to subtle bugs. It is better to use a unique variable name for the type-asserted value.
| if x, ok := x.GetPayload().(*LogEntry_ProtoPayload); ok { | |
| return x.ProtoPayload | |
| } | |
| if p, ok := x.GetPayload().(*LogEntry_ProtoPayload); ok { | |
| return p.ProtoPayload | |
| } |
References
- Linting is done with golangci-lint, which includes checks for variable shadowing. (link)
- Avoid shadowing variables, especially receivers, as it reduces code clarity and can lead to errors.
| if x, ok := x.GetPayload().(*LogEntry_TextPayload); ok { | ||
| return x.TextPayload | ||
| } |
There was a problem hiding this comment.
Shadowing the receiver variable 'x' with the result of a type assertion is confusing. Consider using a different variable name for the asserted value.
| if x, ok := x.GetPayload().(*LogEntry_TextPayload); ok { | |
| return x.TextPayload | |
| } | |
| if p, ok := x.GetPayload().(*LogEntry_TextPayload); ok { | |
| return p.TextPayload | |
| } |
References
- Linting is done with golangci-lint, which flags variable shadowing. (link)
- Avoid shadowing variables to maintain code readability and prevent logic errors.
| if x, ok := x.GetPayload().(*LogEntry_JsonPayload); ok { | ||
| return x.JsonPayload | ||
| } |
There was a problem hiding this comment.
Shadowing the receiver variable 'x' with the result of a type assertion is confusing. Consider using a different variable name for the asserted value.
| if x, ok := x.GetPayload().(*LogEntry_JsonPayload); ok { | |
| return x.JsonPayload | |
| } | |
| if p, ok := x.GetPayload().(*LogEntry_JsonPayload); ok { | |
| return p.JsonPayload | |
| } |
References
- Linting is done with golangci-lint, which flags variable shadowing. (link)
- Avoid shadowing variables to maintain code readability and prevent logic errors.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tiffanny29631 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
1 similar comment
|
/retest |
|
/test all |
2 participants
Reverts #2129 as test failed in Prow