update actions, and fix linting issues #450
Open
Conversation
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
c1dacd9 to
ede6914
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some of these were a slightly false positive, because flagSet.parseLongArg uses a named output var which was already assigned to `err`, and `flagSet.fail()` returns the error-as is; https://github.com/spf13/pflag/blob/10438578954bba2527fe5cae3684d4532b064bbe/flag.go#L935-L942 flag.go:1033:9: Error return value of `f.fail` is not checked (errcheck) f.fail(err) ^ flag.go:1109:9: Error return value of `f.fail` is not checked (errcheck) f.fail(err) ^ This patch just re-assigns it to `err` to keep the linters happy. Many other unhandled errors were either "neglectable" (e.g. errors from `fmt.Printf`), and some unhandled errors would likely cause tests to fail later on, but it doesn't hurt to be explicit and to explicitly mark unhandled errors. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
duration_slice.go:49:12: S1025: should use String() instead of fmt.Sprintf (staticcheck)
out[i] = fmt.Sprintf("%s", d)
^
duration_slice.go:59:9: S1025: should use String() instead of fmt.Sprintf (staticcheck)
return fmt.Sprintf("%s", val)
^
flag.go:658:10: QF1004: could use strings.ReplaceAll instead (staticcheck)
return strings.Replace(s, "\n", "\n"+strings.Repeat(" ", i), -1)
^
flag.go:676:10: QF1004: could use strings.ReplaceAll instead (staticcheck)
return strings.Replace(s, "\n", r, -1)
^
flag.go:688:10: QF1004: could use strings.ReplaceAll instead (staticcheck)
r = r + strings.Replace(l, "\n", "\n"+strings.Repeat(" ", i), -1)
^
flag_test.go:641:5: SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
if flag == nil {
^
flag_test.go:644:10: SA5011: possible nil pointer dereference (staticcheck)
if flag.Name != "boola" {
^
flag_test.go:654:2: SA4006: this value of flag is never used (staticcheck)
flag = f.ShorthandLookup("ab")
^
ipnet_slice_test.go:16:2: S1008: should use 'return c1.String() == c2.String()' instead of 'if c1.String() == c2.String() { return true }; return false' (staticcheck)
if c1.String() == c2.String() {
^
string_array.go:33:2: S1001: should use copy(to, from) instead of a loop (staticcheck)
for i, d := range val {
^
string_array.go:42:2: S1001: should use copy(to, from) instead of a loop (staticcheck)
for i, d := range *s.value {
^
flag.go:658:10: QF1004: could use strings.ReplaceAll instead (staticcheck)
return strings.Replace(s, "\n", "\n"+strings.Repeat(" ", i), -1)
^
flag.go:676:10: QF1004: could use strings.ReplaceAll instead (staticcheck)
return strings.Replace(s, "\n", r, -1)
^
flag_test.go:757:12: QF1004: could use strings.ReplaceAll instead (staticcheck)
result = strings.Replace(result, sep, to, -1)
^
flag_test.go:757:12: QF1004: could use strings.ReplaceAll instead (staticcheck)
result = strings.Replace(result, sep, to, -1)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
bool_test.go:29:22: unnecessary conversion (unconvert)
return triStateValue(*v)
^
count.go:16:18: unnecessary conversion (unconvert)
*i = countValue(*i + 1)
^
uint64.go:30:15: unnecessary conversion (unconvert)
return uint64(v), nil
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
flag_test.go:754:52: replaceSeparators - to always receives "." (unparam)
func replaceSeparators(name string, from []string, to string) string {
^
flag_test.go:1182:24: parseReturnStderr - t is unused (unparam)
func parseReturnStderr(t *testing.T, f *FlagSet, args []string) (string, error) {
^
ip_slice.go:73:56: (*ipSliceValue).fromString - result 1 (error) is always nil (unparam)
func (s *ipSliceValue) fromString(val string) (net.IP, error) {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
bool_func_test.go:53:48: unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _ (revive)
fset.BoolFunc("flag1", "usage message", func(s string) error { return nil })
^
bool_func_test.go:67:72: unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _ (revive)
fset.BoolFunc("flag2", "usage message with `name` placeholder", func(s string) error { return nil })
^
flag_test.go:763:27: unused-parameter: parameter 'f' seems to be unused, consider removing or renaming it as _ (revive)
func wordSepNormalizeFunc(f *FlagSet, name string) NormalizedName {
^
flag_test.go:821:31: unused-parameter: parameter 'f' seems to be unused, consider removing or renaming it as _ (revive)
func aliasAndWordSepFlagNames(f *FlagSet, name string) NormalizedName {
^
flag_test.go:1390:27: unused-parameter: parameter 'f' seems to be unused, consider removing or renaming it as _ (revive)
fs.SetNormalizeFunc(func(f *FlagSet, name string) NormalizedName {
^
func_test.go:67:44: unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _ (revive)
fset.Func("flag1", "usage message", func(s string) error { return nil })
^
golangflag.go:74:6: exported: func name will be used as pflag.PFlagFromGoFlag by other packages, and that stutters; consider calling this FromGoFlag (revive)
func PFlagFromGoFlag(goflag *goflag.Flag) *Flag {
^
ipnet_slice_test.go:11:14: unused-parameter: parameter 'ip' seems to be unused, consider removing or renaming it as _ (revive)
func getCIDR(ip net.IP, cidr *net.IPNet, err error) net.IPNet {
^
time.go:54:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
} else {
return d.Time.Format(time.RFC3339Nano)
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
601718a to
6f56b92
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
6f56b92 to
9e20f80
Compare
1 task
- set default permissions to "content: read" as actions in this repository
don't require write acccess.
- set a guardrails timeout; this should not be needed normally, but
GitHub's default timeout is 6 Hours, and prevents runaway actions
that may happen occasionally on GitHub service outages.
- remove filter for branch name and run on pull-requests, push events
- add "stable" and "oldstable" Go versions to the matrix; these always
point to the currently maintained versions of Go (currently Go1.25
and Go1.24). Using the fixed ("oldstable", "stable") keywords reduces
maintainance and helps marking checks as "required" in branch protection,
as such options are using the name generated from the matrix.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
9e20f80 to
597f535
Compare
tomasaschan
reviewed
Sep 4, 2025
Comment on lines
+5
to
+14
| - errorlint | ||
| - nolintlint | ||
| - revive | ||
| - unconvert | ||
| - unparam | ||
| exclusions: | ||
| generated: disable | ||
| presets: | ||
| - comments | ||
| - std-error-handling |
There was a problem hiding this comment.
Why this specific set of linters and presets? Is it based on what the repo is currently configured with, or on some listed standard set somewhere, or just your personal preference?
I personally don't mind being very specific with what code should look like - I think standardizing formatting from the get-go was one of the best things of Go when it launched - so if there's a way to just say "enable everything" I think it'd be fine to use that instead of listing specific linters or rulesets.
| } | ||
|
|
||
| return nil, fmt.Errorf("invalid string being converted to Bytes: %s %s", sval, err) | ||
| return nil, fmt.Errorf("invalid string being converted to Bytes: %s %s", sval, err.Error()) |
There was a problem hiding this comment.
Is %w also a not-in-1.12 thing?
| if len(name) > 1 { | ||
| msg := fmt.Sprintf("can not look up shorthand which is more than one ASCII character: %q", name) | ||
| fmt.Fprintf(f.Output(), msg) | ||
| _, _ = fmt.Fprint(f.Output(), msg) |
There was a problem hiding this comment.
Contrary to my call to enable everything above, I actually think this one smells pretty badly (stinks?)... Could we please not require a bunch of needless syntax when printing stuff?
Comment on lines
-1035
to
+1034
| f.fail(err) | ||
| err = f.fail(err) |
There was a problem hiding this comment.
This will change what error is returned by the function. Is that correct?
| Set("test_duration", "1s") | ||
| Set("test_optional_int", "1") | ||
| if err := Set("test_bool", "true"); err != nil { | ||
| t.Error(err) |
There was a problem hiding this comment.
Does this support passing nil (and, in that case, not fail)? If so, we could get rid of all the branches.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
See individual commit messages for details.