Skip to content

Conversation

@infinisil
Copy link
Member

@infinisil infinisil commented Apr 26, 2024

We shouldn't use personal access tokens, instead we created a GitHub App with read-only access to just this repository.

While codeowners-validator supports GitHub App authentication, the same cannot be said for the hacky script I wrote because there was no support for checking write access: mszostok/codeowners-validator#157

Instead of trying to hack the script more to make it work with GitHub App authentication, I decided to implement it into codeowners-validator itself: mszostok/codeowners-validator#222

Because it's not merged/released yet, we need to build it ourselves, so I added some Nix to do that reproducibly.

Testing this in #9

@infinisil
Copy link
Member Author

We can't make CI pass here because it uses the base branch's workflow file, which doesn't use GitHub Apps yet.

However I tested this in a separate PR where I can confirm that it works: #9

@infinisil infinisil marked this pull request as ready for review April 26, 2024 17:47
We shouldn't use personal access tokens, instead we created a GitHub App
with read-only access to just this repository.

While codeowners-validator supports GitHub App authentication,
the same cannot be said for the hacky script I wrote because there was no support
for checking write access: mszostok/codeowners-validator#157

Instead of trying to hack the script more to make it work with GitHub App authentication,
I decided to implement it into codeowners-validator itself: mszostok/codeowners-validator#222

Because it's not merged/released yet, we need to build it ourselves,
so I added some Nix to do that reproducibly.
@zimbatm zimbatm merged commit 0d96812 into main Apr 26, 2024
@zimbatm zimbatm deleted the github-app branch April 26, 2024 19:04
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.

2 participants