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

feat: support github enterprise api #225

Merged

Conversation

ricardojdsilva87
Copy link
Contributor

Pull Request

Proposed Changes

This Pull request is based on the already merged github/evergreen#256 to give support to GitHub Enterprise login using a GitHub App also created on GitHub Enterprise

  • Split the authentication to a separate file
  • Added needed tests

Code coverage

---------- coverage: platform darwin, python 3.13.0-final-0 ----------
Name             Stmts   Miss  Cover   Missing
----------------------------------------------
auth.py             16      1    94%   47
stale_repos.py     151     16    89%   142-143, 184-185, 200-201, 224, 326, 328, 335-336, 394-395, 402-403, 413
----------------------------------------------
TOTAL              167     17    90%

Required test coverage of 80% reached. Total coverage: 89.82%

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing
  • If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from @jeffrey-luszcz

Reviewer

  • Label as either fix, documentation, enhancement, infrastructure, maintenance or breaking

Copy link
Member

@jmeridth jmeridth left a comment

Choose a reason for hiding this comment

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

One import question. Nice break out of the auth content.

stale_repos.py Outdated Show resolved Hide resolved
@jmeridth
Copy link
Member

jmeridth commented Jan 5, 2025

@ricardojdsilva87
Copy link
Contributor Author

new linting errors to handle

Hello @jmeridth regarding these lint errors the only way I found was to update the isort.cfg file to have:

[settings]
profile = black
known_third_party = github3,dateutil,dotenv
known_first_party = auth

Otherwise running locally make lint is always organizing the imports in the wrong way that triggers the error. Should I add this or should this be done in a different way?
Thanks

@jmeridth
Copy link
Member

jmeridth commented Jan 6, 2025

new linting errors to handle

Hello @jmeridth regarding these lint errors the only way I found was to update the isort.cfg file to have:

[settings]
profile = black
known_third_party = github3,dateutil,dotenv
known_first_party = auth

Otherwise running locally make lint is always organizing the imports in the wrong way that triggers the error. Should I add this or should this be done in a different way? Thanks

I'm good with this change. Thank you for fixing.

@jmeridth
Copy link
Member

jmeridth commented Jan 6, 2025

To fix those linting errors you can add them to the configuration.

disable=raw-checker-failed,
        bad-inline-option,
        locally-disabled,
        file-ignored,
        suppressed-message,
        useless-suppression,
        deprecated-pragma,
        use-symbolic-message-instead,
        use-implicit-booleaness-not-comparison-to-string,
        use-implicit-booleaness-not-comparison-to-zero,
        import-error
        line-too-long
        too-many-arguments

@jmeridth jmeridth merged commit d6c4576 into github:main Jan 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants