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

nit(scm): group functions by class they come from in main integration class #76240

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

cathteng
Copy link
Member

@cathteng cathteng commented Aug 14, 2024

The __Integration classes (e.g. GitHubIntegration) inherit from several classes (ABCs), and they each implement functions from those parent classes. This PR moves functions such that they are grouped together via the ABC they come from, e.g. IntegrationInstallation functions or RepositoryIntegration functions.

@cathteng cathteng requested a review from a team August 14, 2024 22:55
@cathteng cathteng requested a review from a team as a code owner August 14, 2024 22:55
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 14, 2024
@@ -362,6 +353,18 @@ def extract_source_path_from_source_url(self, repo: Repository, url: str) -> str
return qs["path"][0].lstrip("/")
return ""

# Azure DevOps only methods

def _check_domain_name(self, default_identity: RpcIdentity) -> None:
Copy link
Member Author

Choose a reason for hiding this comment

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

this is only used inside this class so i added an underscore

Copy link
Member

@iamrajjoshi iamrajjoshi left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 72.34043% with 13 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files Patch % Lines
...ntry/integrations/github_enterprise/integration.py 14.28% 6 Missing ⚠️
src/sentry/integrations/gitlab/integration.py 57.14% 1 Missing and 2 partials ⚠️
src/sentry/integrations/vsts/integration.py 88.88% 3 Missing ⚠️
...entry/integrations/bitbucket_server/integration.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #76240       +/-   ##
===========================================
+ Coverage   46.81%   88.40%   +41.59%     
===========================================
  Files        1891     2968     +1077     
  Lines      122785   185089    +62304     
  Branches    22108    32982    +10874     
===========================================
+ Hits        57487   163637   +106150     
+ Misses      64212    15463    -48749     
- Partials     1086     5989     +4903     

@cathteng cathteng requested a review from a team August 15, 2024 15:38
@cathteng cathteng merged commit 987941b into master Aug 15, 2024
50 of 51 checks passed
@cathteng cathteng deleted the cathy/scm/organize-integration-methods branch August 15, 2024 16:38
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants