Initial Content #1
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
mmlb
force-pushed
the
initial-content
branch
3 times, most recently
from
8f0ae49 to
8ec9bae
Compare
mmlb
force-pushed
the
initial-content
branch
4 times, most recently
from
c596299 to
5fca3e0
Compare
|
Not sure if I want to drop gomnd and wsl in this PR or a follow up, I think follow up...? |
|
@mmlb, would prefer if this was based this off https://github.com/metal-toolbox/mctl/blob/main/.golangci.yml#L1-L90 |
mmlb
force-pushed
the
initial-content
branch
3 times, most recently
from
71eac80 to
baf9841
Compare
|
Alright @joelrebel, @DoctorVin I've updated the config file to be based off of mctl's. There were some invalid configs, cargo culted settings and unnecessary disables that I've removed/tweaked. Both mctl and ironlib will need some tweaks to pass with this config but imo they are worth doing. mctl is pretty good, ironlib needs the most work (mostly mechanical). I'll post a PR to each with the changes necessary to work with this config in a little bit. It would probably be easier to review each commit on its own, I made it so it would be easier to trust the changes/diff if reviewed that way. |
|
mctl: metal-toolbox/mctl#108 |
mmlb
force-pushed
the
initial-content
branch
2 times, most recently
from
0d53c40 to
ec59e57
Compare
Use prettier just to keep the files looking nice and catch any json/yaml issues.
Importing as-is in the PR and will tweak further soon. Imported from commit: 9d20393342f72e2d12083d92bf8c32e861b22038.
Just sorting things so I can find things quicker and remove dupes (misspell & staticcheck).
Config verify suppor came along in golangci-lint@v1.57.
These are reported by new feature that landed in golangci-lint v1.57: `config verify`. I verified that there's no errors reported in current mctl while messing with this file. Some of the features were removed instead of put into the right place since they were not actually being used and thus unnecessary. In some cases I needed to figure out the correct setting/tweak and referred to [1] for some more information. Here's the list of errors reported by golangci-lint config verify before these fixes: ``` go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 config verify --config .golangci.yml WARN [config_reader] The configuration option `run.skip-files` is deprecated, please use `issues.exclude-files`. WARN [config_reader] The configuration option `run.skip-dirs` is deprecated, please use `issues.exclude-dirs`. WARN [config_reader] The configuration option `linters.govet.check-shadowing` is deprecated. Please enable `shadow` instead, if you are not using `enable-all`. jsonschema: "linters-settings.govet" does not validate with "/properties/linters-settings/properties/govet/additionalProperties": additionalProperties 'auto-fix', 'check-shadowing' not allowed jsonschema: "linters-settings.depguard" does not validate with "/properties/linters-settings/properties/depguard/additionalProperties": additionalProperties 'list-type' not allowed jsonschema: "linters-settings.revive" does not validate with "/properties/linters-settings/properties/revive/additionalProperties": additionalProperties 'min-confidence' not allowed jsonschema: "linters-settings.whitespace" does not validate with "/properties/linters-settings/properties/whitespace/additionalProperties": additionalProperties 'auto-fix' not allowed jsonschema: "linters-settings.misspell" does not validate with "/properties/linters-settings/properties/misspell/additionalProperties": additionalProperties 'auto-fix' not allowed jsonschema: "run" does not validate with "/properties/run/additionalProperties": additionalProperties 'skip-dirs', 'skip-files' not allowed jsonschema: "" does not validate with "/additionalProperties": additionalProperties 'exclude-use-default', 'service' not allowed Error: the configuration contains invalid elements ```` 1: https://github.com/golangci/golangci-lint/blob/v1.57.0/.golangci.reference.yml
Some of these seem to apploy to the golangci-lint repository itself not any of ours. The excludes did not affect mctl nor ironlib so lets drop it.
Just so its not lost following the long list.
No changes to enabled/disabled linters though, checked with ``` go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 linters >before go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 linters >after diff -u before after ````
These are all happy with mctl and only some of them complain in ironlib. We should make ironlib pass these linters and its not too much of a chore. I had to bump dupl.threshold because it was complaining about "duplicate" code in NewConditionsClient and NewBomServiceClient. I don't agree, but also don't really want to drop dupl just yet so lets see if bumping works. This config won't apply right away to mctl but the changes needed are very minor/easy fixes that should definitely be committed. Its mostly gofumpt & gci (which are fixed by running golangci-lint run --fix) and some typos.
Test files are go files to and deserve to be legible. They also already pass all the linters that the "real" code passes. There are some test only linters that are disabled due to ironlib. We should fix ironlib so it passes as the failing linters are actually useful.
joelrebel
approved these changes
Apr 15, 2024
There was a problem hiding this comment.
Thanks for the efforts here.
Before we PR changes to the other projects, we can have this repo provide a reusable GH workflow that others import. And the reusable workflow could verify a project linter configuration does not differ from the one here. I can take this up.
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.
No description provided.