Skip to content

Conversation

@ldez
Copy link
Member

@ldez ldez commented Apr 24, 2021

The purpose of this cleaning is to facilitate contribution and maintenance.

The PR provides a homogeneous design:

  • each configuration section have a dedicated file
  • no anonymous structure
  • the linters in the main Config structure are sorted alphabetically and the linters settings structures are also sorted alphabetically.

I created one commit by change like that it's easier to review.

@ldez ldez added topic: cleanup Related to code, process, or doc cleanup area: config Related to .golangci.yml and/or cli options labels Apr 24, 2021
Copy link
Member

@bombsimon bombsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks much better structured and sorted! 🎉

Not sure the last commit is really provides increased maintainability, to me it's a bit stuttering to prefix every file in a directory with the same name (config/config_*) but I guess that's just personal preference.

Yes, I saw that reader.go was left but tbh I don't really see why this one wouldn't be prefixed as well since it's reading/parsing config.

@ldez
Copy link
Member Author

ldez commented Apr 24, 2021

@bombsimon I added the last commit because the file reader.go is not a part of the Config struct, it can be confusing.
I'm also not a fan of the prefix config_ in the package config, but I didn't find a better idea.

Edit: I had not seen your very small comment 😄

@bombsimon
Copy link
Member

bombsimon commented Apr 24, 2021

I have no strong feeling hence the approve! If you think this is better than keeping the names go ahead and merge or wait for an additional review. I'm just appreciating your effort in maintaining this project!

@ldez
Copy link
Member Author

ldez commented Apr 24, 2021

I think I will drop the last commit because we are, at least you and me, not to be a fan of the prefix.

@ldez ldez force-pushed the feat/cleanup-config branch from be1aba3 to 8bae52a Compare April 24, 2021 14:19
@ldez ldez merged commit db80e16 into golangci:master Apr 24, 2021
@ldez ldez deleted the feat/cleanup-config branch April 24, 2021 14:29
@ldez ldez added this to the v1.40 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: config Related to .golangci.yml and/or cli options topic: cleanup Related to code, process, or doc cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants