etcd-io
[3.4] installed revive on CI and added doc.go to fix revive linter errors
This is meant to address the failing revive checks mentioned in this issue: https://github.com/etcd-io/etcd/issues/17472
Screenshot of Revive linter errors:
Output with code changes:
Hi @thedtripp. Thanks for your PR.
I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
This looks good overall. I wonder if a better approach would be to add a doc.go file to the rest of the packages that don't have the module documentation. However, I also don't know if that's something we want to do for 3.4 or if we should go with the current approach. Thoughts, @jmhbnz?
/retitle [3.4] formatting: added package comments to fix revive linter errors.
This looks good overall. I wonder if a better approach would be to add a
doc.gofile to the rest of the packages that don't have the module documentation. However, I also don't know if that's something we want to do for 3.4 or if we should go with the current approach. Thoughts, @jmhbnz?
This decision I think is best to be made by @ahrtr and/or @serathius who were around for much more of 3.4 development than I was. Personally I would lean towards adding missing doc.go files but that might be overkill.
https://github.com/etcd-io/etcd/issues/17472#issuecomment-2198111342
Please ensure the linter isn't skipped firstly, then resolve the errors detected by the linter.
Personally I would lean towards adding missing
doc.gofiles
OK for me. Actually most of the packages already have doc.go files.
Please ensure the linter isn't skipped firstly, then resolve the errors detected by the linter.
@ahrtr Thanks for the review. Should this be done in one PR, or two separate ones?
@ahrtr Thanks for the review. Should this be done in one PR, or two separate ones?
Please do it in one PR, you can have one or two commits (the first commit ensures the linter isn't skipped; the second commit fixes the error raised by the linter).
/retitle [3.4] installed revive on CI and added doc.go to fix revive linter errors.
@thedtripp: Re-titling can only be requested by trusted users, like repository collaborators.
In response to this:
/retitle [3.4] installed revive on CI and added doc.go to fix revive linter errors.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
/retitle [3.4] installed revive on CI and added doc.go to fix revive linter errors
@thedtripp: How can you tell a vampire has a cold? They start coffin.
In response to this:
/joke
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
This looks great, @thedtripp. Would it be possible to add:
This is based on commit https://github.com/thedtripp/etcd/commit/4f238837aa1945919b197f300fe7fe244eac4d8c and pull request https://github.com/etcd-io/etcd/pull/14872.To the backporting commit? Thanks!
@ivanvc Done. Do you know anything about the failing go vulnerability checker. I'm about to start looking into it out of curiosity. https://github.com/etcd-io/etcd/actions/runs/9771618251/job/26974642695?pr=18215
Edit:
Denial of service due to improper 100-continue handling in net/http
More info: https://pkg.go.dev/vuln/GO-2024-2963
Standard library
Found in: net/[email protected]
Fixed in: net/[email protected]
Example traces found:
Error: #1: tests/e2e/etcd_process.go:296:28: e2e.BinaryFailpoints.DeactivateHTTP calls http.Client.Do
Error: #2: etcdserver/cluster_util.go:75:22: etcdserver.getClusterFromRemotePeers calls http.Client.Get
Error: #3: tools/etcd-dump-metrics/install_linux.go:35:23: etcd.install calls http.Get
Error: #4: etcdserver/api/rafthttp/transport.go:229:26: rafthttp.Transport.Stop calls http.Transport.CloseIdleConnections
Error: #5: pkg/transport/transport.go:70:32: transport.unixTransport.RoundTrip calls http.Transport.RoundTrip
Your code is affected by 1 vulnerability from the Go standard library.
This scan found no other vulnerabilities in packages you import or modules you
require.
Use '-show verbose' for more details.
Error: Process completed with exit code 3.```
@ivanvc Done. Do you know anything about the failing go vulnerability checker. I'm about to start looking into it out of curiosity. https://github.com/etcd-io/etcd/actions/runs/9771618251/job/26974642695?pr=18215
Please refer to #18269 (which I just opened :sweat_smile:).
I just noticed I put 2014 as the date in clientv3/internal/resolver/doc.go. I'll fix that and squash.