Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename to codeowners, extend configuration capabilities, add pre-commit-hook support #199

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bobertrublik
Copy link

@bobertrublik bobertrublik commented Jun 14, 2023

Description

This PR changes a few issues at once which I found while getting to know the project. In order to allow the use of codeowners-validator as pre-commit hook a lot of changes were necessary. As described in #140 pre-commit hooks do not really work with env variables and flags are currently not supported by the code base. Additionally in #116 it was considered to rename codeowners-validator to codeowners in order to make the project more future proof for further functions.

Changes proposed in this pull request:

  • rename codeowners-validator to codeownersand create a validate subcommand which inhibits the current capabilities of this project
  • replace envconfig with cobra and viper
  • add support for flags and config files. environment variables require the prefix CODEOWNERS_.
  • place api and config into own packages
  • add pre-commit-hooks.yaml file

Related issue(s)

Additional information

I fixed most of the integration tests but can't run the ones which require an API key. The other tests run fine.

One issue I wasn't able to fix is in the commit c372b63. I have no idea how to add a custom type as flag since I need to convert it somewhere. I would appreciate help there :) Other than that it's very possible I made some errors or mistakes which could be solved more elegantly. Feedback is very welcome!

This PR also includes #197.

@bobertrublik
Copy link
Author

I tried to rebase and I ran into a lot of conflicts. I can fix them but first I wanted to ask, if you'd prefer to make this PR perhaps a little smaller? I can drop the renaming to codeowners if you prefer. Also I'd still have to take a look how to accept custom types as a flag. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant