Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

feat: support github enterprise api#142

Merged
zkoppert merged 2 commits intogithub:mainfrom
ricardojdsilva87:feat/support-github-enterprise-api
Nov 4, 2024
Merged

feat: support github enterprise api#142
zkoppert merged 2 commits intogithub:mainfrom
ricardojdsilva87:feat/support-github-enterprise-api

Conversation

@ricardojdsilva87
Copy link
Copy Markdown
Contributor

@ricardojdsilva87 ricardojdsilva87 commented Oct 30, 2024

Pull Request

Proposed Changes

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

  • Added the file .coveragerc to ignore the python test files, because of that the coverage went below the limit of 60% and still need to check if there is the possibility to add tests for the open_contrib_pr.py code

Code coverage

---------- coverage: platform darwin, python 3.13.0-final-0 ----------
Name                 Stmts   Miss  Cover   Missing
--------------------------------------------------
auth.py                 28      4    86%   48, 78-80
env.py                  60      3    95%   26, 45-46
open_contrib_pr.py      63     63     0%   4-105
--------------------------------------------------
TOTAL                  151     70    54%

FAIL Required test coverage of 60% not reached. Total coverage: 53.64%

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 bug, documentation, enhancement, infrastructure, maintenance or breaking

Copy link
Copy Markdown
Contributor

@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.

LGTM just one question

jmeridth
jmeridth previously approved these changes Nov 1, 2024
@jmeridth jmeridth dismissed their stale review November 1, 2024 12:47

failing CI. Just saw it. Code looks good otherwise.

@zkoppert
Copy link
Copy Markdown
Member

zkoppert commented Nov 4, 2024

RE: test failure. Test coverage is too low.

If @jmeridth you feel like the added code has decent coverage for what was changed we can bypass this check. I'll open a new pr to add some tests. Looks like one file has 0% coverage.

@jmeridth
Copy link
Copy Markdown
Contributor

jmeridth commented Nov 4, 2024

RE: test failure. Test coverage is too low.

If @jmeridth you feel like the added code has decent coverage for what was changed we can bypass this check. I'll open a new pr to add some tests. Looks like one file has 0% coverage.

I'm comfortable for us to merge this. @zkoppert you?

@zkoppert zkoppert merged commit a716356 into github:main Nov 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants