Enable golangci-lint for build monitor #6012
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Nino Kodabande <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
check-spelling found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
Nino-K
force-pushed
the
enable-golangci
branch
from
26ea840
to
4cec300
Compare
Nino-K
force-pushed
the
enable-golangci
branch
8 times, most recently
from
a4cc9b1
to
7810309
Compare
mook-as
requested changes
Nov 30, 2023
.github/workflows/.golangci.yaml
Outdated
| nolintlint: | ||
| allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) | ||
| allow-unused: false # report any unused nolint directives | ||
| require-explanation: false # don't require an explanation for nolint directives |
There was a problem hiding this comment.
I'd like us to turn this on (but not in this PR); same as the next line.
| sparse-checkout: src/go/github-runner-monitor | ||
| sparse-checkout: | | ||
| src/go/github-runner-monitor | ||
| .github |
There was a problem hiding this comment.
Suggested change
| .github | |
| .github/workflows # For .golangci.yaml |
.github/workflows/.golangci.yaml
Outdated
There was a problem hiding this comment.
Would this get picked up as an (invalid) workflow?
Consider putting this in a subdirectory instead, e.g. .github/workflows/data/.golangci.yaml. (Workflows never pick up files in subdirectories.)
| working-directory: src/go/github-runner-monitor | ||
| only-new-issues: true | ||
| only-new-issues: true |
| @@ -30,6 +30,8 @@ import ( | |||
| "golang.org/x/sys/unix" | |||
| ) | |||
|
|
|||
| var timeout = 30 | |||
There was a problem hiding this comment.
Suggested change
| var timeout = 30 | |
| var timeout = 30 * time.Second |
That makes it clearer what unit this is in (and fixes the cast below).
Also, shouldn't this be a const?
Nino-K
force-pushed
the
enable-golangci
branch
from
7810309
to
2a2c78f
Compare
Nino-K
force-pushed
the
enable-golangci
branch
5 times, most recently
from
9f666bc
to
9105ab3
Compare
Signed-off-by: Nino Kodabande <[email protected]>
Nino-K
force-pushed
the
enable-golangci
branch
from
9105ab3
to
a1c15f9
Compare
mook-as
approved these changes
Nov 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a
.golangci.yamlthat will be shared across all golang repos undersrc/go. Also, enables the configuration argument for theGitHub Runner: Build Monitor.Related: #5919