gha: extend matrix to include Windows and macOS, and lint on all #451
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
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
- 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). Signed-off-by: Sebastiaan van Stijn <[email protected]>
Author
|
edit: nope; it's line-endings; see below Also looks like go1.12 is not available for macOS and Windows, so we may need to skip that combination. |
thaJeztah
force-pushed
the
update_gha_matrix
branch
3 times, most recently
from
6293265 to
f29ea5f
Compare
Author
|
Still failing on Windows; I suspect it's a newlines issue; ISTR we ran into that in some repository, but not sure if we found a workaround at the time 🤔 |
gofmt and other tools expect Go files to use Unix line-endings, and may fail on Windows when using CRLF. Add a .gitattributes to force using Unix line endings for text-files in this repository. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Combine "Lint" and "Test" into a single matrix so that linting runs on all versions of Go and all platforms. Some linters apply rules depending on Go version and we must avoid applying rules that are not applicable to older versions of Go that are supported by this module. Combining "Lint" and "Test" allows re-using the common steps, such as checking out the code and setting up Go; duration of tests and linting on this repository is short, so running Lint and Test in parallel does not gain much time. This patch also reduces the test matrix to "oldest supported version of Go (by this module), and currently supported versions of Go (current stable, and previous stable)". Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
update_gha_matrix
branch
from
1e1ed22 to
287815e
Compare
Author
|
Ah, yes; it was just adding a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
gha: extend matrix to include Windows and macOS, and lint on all
Combine "Lint" and "Test" into a single matrix so that linting runs
on all versions of Go and all platforms. Some linters apply rules
depending on Go version and we must avoid applying rules that are
not applicable to older versions of Go that are supported by this
module.
Combining "Lint" and "Test" allows re-using the common steps, such
as checking out the code and setting up Go; duration of tests and
linting on this repository is short, so running Lint and Test in
parallel does not gain much time.
This patch also reduces the test matrix to "oldest supported version
of Go (by this module), and currently supported versions of Go (current
stable, and previous stable)".
Using the fixed ("oldstable", "stable") keywords helps marking checks
as "required" in branch protection, as such options are using the name
generated from the matrix.