Adding more checks for team permissions. Adding more ENV variables

mszostok / codeowners-validator

The GitHub CODEOWNERS file validator

Apache License 2.0

219 stars 47 forks source link

Adding more checks for team permissions. Adding more ENV variables #4

Closed njegosrailic closed 4 years ago

njegosrailic commented 4 years ago

Adding more checks for a team permissions According to https://github.community/t5/How-to-use-Git-and-GitHub/CODEOWNERS-works-with-users-but-not-teams/td-p/4986 a team needs write access to a repository.
Adding environment variables to be able to run file check and code owners permissions check separately. We have big code owners files and it takes a lot of time. Also we are hitting API rate limits.
Fix the error message in validateTeam function

mszostok commented 4 years ago

@njegosrailic I will also try to check the GitHub GraphQL API and maybe I will be able to replace the rest calls with graphql queries, as a result I will be able to reduce the problems with hitting API GitHub quota.

btw. If you do not have time then we can merge your pull request and I will adjust the selective testing in this pr: https://github.com/mszostok/codeowners-validator/pull/8

njegosrailic commented 4 years ago

Hey @mszostok ,

Thank you very much for all the information you provided and suggestions how to improve the code. :) I'll update my PR soon. My apologies but I'm very busy these days.

Also, we are going to try: https://github.com/mszostok/codeowners-validator/compare/check-enabled-disabled?expand=1 for other services not primary for code owners.

I have checked with github support and they have no any official tool to validate code owners.

mszostok commented 4 years ago

I have checked with GitHub support and they have no official tool to validate code owners.

yep, this is true and this was also the reason that I created such a tool. In the upcoming week, I want to do the release and use that officially in Kyma project created by SAP.

so new improvements will come soon

My apologies but I'm very busy these days.

no problem :)

mszostok commented 4 years ago

FYI: CI checks failed because the imports are not sorted

You can execute those checks also locally: ./hack/ci/run-lint.sh ./hack/ci/run-tests.sh

In few days I will update documentation and add developer guide:)

njegosrailic commented 4 years ago

FYI: CI checks failed because the imports are not sorted

You can execute those checks also locally: ./hack/ci/run-lint.sh ./hack/ci/run-tests.sh

In few days I will update documentation and add developer guide:)

Actually I accidentally dropped an line. It should be all set. The problem is because I have opened the PR from my forked repo and I did some changes there to be able to compile it. Once you merge the change will drop my fork and migrate config to yours.

mszostok commented 4 years ago

@njegosrailic and what about your feature with checking the user permission? Right now it’s not included in this pr

You want to submit it in separate pr?

njegosrailic commented 4 years ago

@njegosrailic and what about your feature with checking the user permission? Right now it’s not included in this pr

You want to submit it in separate pr?

Added that back.

mszostok commented 4 years ago

Thanks @njegosrailic for your PR!

The new release with your change is already available on the GitHub: https://github.com/mszostok/codeowners-validator/releases/tag/v0.2.0

Check the release notes for more detailed info 🙂