Current coverage is 49.00% (diff: 100%)

Merging #14 into master will not change coverage

@@ master #14 diff @@
==========================================
 Files 9 9 
 Lines 200 200 
 Methods 0 0 
 Messages 0 0 
 Branches 0 0 
==========================================
 Hits 98 98 
 Misses 102 102 
 Partials 0 0 

Powered by Codecov. Last update 397a658...fa7c4b8

@icedfish repo is requested right now because sentry-github needs it. We could probably be smarter about this, but I feel like the only way we could really do that is by having sentry-auth-github and sentry-github being the same project.

Is not the sane thing to do here to check for GITHUB_EXTENDED_PERMISSIONS as used elsewhere and if it exists and is empty then don't include the repo scope and obviously if it has repo then ask for repo perms (and I guess potentially others)?

GITHUB_EXTENDED_PERMISSIONS = [] # don't ask for repo perms
GITHUB_EXTENDED_PERMISSIONS = ['repo'] # do ask for repo perms

Otherwise behave as-is

Just a thought..

You could really improve this changing that line with:

SCOPE = ','.join(filter(None, ['user:email', getattr(settings, 'GITHUB_REQUIRE_PERMS', None)]))

In some cases Gh is used just for login credentials.

Cheers

Given these two plugins are now separate, and this scope has no effect on github integration, I don't see a reason why this should not be merged. I've been running this for a while and it works perfectly fine.

Can this contribution be reviewed? I think it's an interesting feature and merging it would avoid some forks. Thank you.