search

tinkerbell / lint-install

Consistently install reasonable linter rules for open-source projects
Apache License 2.0
6 stars 7 forks source link

Change default filename from .golangci.yaml to .golangci.yml #30

Closed mmlb closed 2 years ago

mmlb commented 2 years ago

Description

Changes the filename used for golangci-lint configuration. Also deletes any of the other golangci-lint config files if they exist, to avoid confusion.

Why is this needed

The file name used changed when a refactor happened in a4c0feb21cf9ad350543163d4f75725f8626e93c. Before: https://github.com/tinkerbell/lint-install/commit/a4c0feb21cf9ad350543163d4f75725f8626e93c#diff-845a2bf631b9895ecaf80c8db474db3d620a15d19171b4d6f7ee030bbe21a580L149 After: https://github.com/tinkerbell/lint-install/commit/a4c0feb21cf9ad350543163d4f75725f8626e93c#diff-845a2bf631b9895ecaf80c8db474db3d620a15d19171b4d6f7ee030bbe21a580R272

But the old file was never cleaned up, which could have led to having 2 golangci-lint config files on disk and possible confusion. This ends up moving us back to the original file which is the first listed in https://golangci-lint.run/usage/configuration/#config-file and seems to be the preferred one. We also proactively avoid any other golangci-lint config files on disk to avoid confusion.

How Has This Been Tested?

Ran lint-install and saw the correct file being written. I've also verified that golangci.yaml is deleted if it was pre-existing and that lint-install works correctly if it doesn't exist.

How are existing users impacted? What migration steps/scripts do we need?

golangci-lint config is used as intended, it might complain about issues due to new settings.

mmlb commented 2 years ago

@tstromberg should we also remove .toml and .json versions of the file? Now that its not fixing a mistake and instead bringing things into a standardized name it sort of makes sense to ensure the other file names don't exist.

mmlb commented 2 years ago

@tstromberg should we also remove .toml and .json versions of the file? Now that its not fixing a mistake and instead bringing things into a standardized name it sort of makes sense to ensure the other file names don't exist.

Also if we do do this, then the renaming message doesn't really make sense.

mmlb commented 2 years ago

@tstromberg should we also remove .toml and .json versions of the file? Now that its not fixing a mistake and instead bringing things into a standardized name it sort of makes sense to ensure the other file names don't exist.

Also if we do do this, then the renaming message doesn't really make sense.

Maybe standardizing on golangci.yml, deleting $file?

mmlb commented 2 years ago

I'll go with using golangci.yml instead of $filename for consistency

mmlb commented 2 years ago

I've updated the PR and description by taking out any wording about the filename being incorrect and replaced it with avoiding confusion. I've also added the deletion of the other golangci-lint config files if they exist to proactively avoid confusion.

mmlb commented 2 years ago

Python packagin is failing the build I think: https://github.com/tinkerbell/lint-install/runs/3873177171#step:7:33

ERROR: launchpadlib 1.10.13 requires testresources, which is not installed.

mmlb commented 2 years ago

Nope my own mistake!