Conversation
Ignored code generated files and fixed the left checks Signed-off-by: JUN JIE NAN <nanjunjie@gmail.com>
stgraber
reviewed
Jul 23, 2024
| "golang.org/x/text/language" | ||
|
|
||
| "github.com/lxc/incus/v6/client" | ||
| incus "github.com/lxc/incus/v6/client" |
Member
There was a problem hiding this comment.
Looks like you ran goimports or something like that which has a tendency to go mess with our import names, can you remove all of those from your commit?
stgraber
reviewed
Jul 23, 2024
Comment on lines
-430
to
+432
| } else { //nolint:staticcheck // (keep the empty branch for the comment) | ||
| //nolint:all | ||
| //lint:ignore SA9003 keep the empty branch for the comment | ||
| } else { //nolint:all |
Member
There was a problem hiding this comment.
Not sure what's going on here, that change looks off.
stgraber
reviewed
Jul 23, 2024
| content := fmt.Sprintf(`%spackage %s | ||
|
|
||
| // The code below was generated by %s - DO NOT EDIT! | ||
| //lint:file-ignore U1000,SA4006 Ignore staticcheck |
Member
There was a problem hiding this comment.
So just because the files are auto-generated doesn't mean that issues/lints aren't worth fixing.
It just means that rather than fixing the issues directly in the .mapper.go files, those issues need to instead be fixed in the generator code.
stgraber
reviewed
Jul 23, 2024
| // Package response contains helpers for rendering HTTP responses. | ||
| // | ||
| //nolint:deadcode,unused | ||
| //nolint:all |
stgraber
reviewed
Jul 23, 2024
| // Golang has deprecated all methods relating to PEM encryption due to a vulnerability. | ||
| // However, the weakness does not make PEM unsafe for our purposes as it pertains to password protection on the | ||
| // key file (client.key is only readable to the user in any case), so we'll ignore deprecation. | ||
| //nolint:all |
stgraber
reviewed
Jul 23, 2024
| return nil, fmt.Errorf("Unsupported key type: %T", sshKey) | ||
| } | ||
| } else { | ||
| //nolint:all |
stgraber
reviewed
Jul 23, 2024
| // Don't lint next line with gosimple. It says we should convert reqValidate directly to an RecoverImportPost | ||
| // because their types are identical. This is less clear and will not work if either type changes in the future. | ||
| reqImport := recover.ImportPost{ //nolint:gosimple | ||
| //nolint:all |
stgraber
reviewed
Jul 23, 2024
| // Wait for heartbeat to finish and then release. | ||
| // Ignore staticcheck "SA2001: empty critical section" because we want to wait for the lock. | ||
| gateway.HeartbeatLock.Lock() | ||
| //nolint:all |
stgraber
reviewed
Jul 23, 2024
| // Ignore staticcheck "SA2001: empty critical section" because we want to wait for the lock. | ||
| gateway.HeartbeatLock.Lock() | ||
| //nolint:all | ||
| //lint:ignore SA2001 we want to wait for the lock |
Member
There was a problem hiding this comment.
The comment above (// Ignore staticcheck ...) can be removed given this one just repeats it
This file contains hidden or 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
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.
Ignored code generated files and fixed the left checks