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

refactor: improve fetching repositories with custom properties #742

Open
wants to merge 1 commit into
base: main-enterprise
Choose a base branch
from

Conversation

PendaGTP
Copy link
Contributor

Description

Related to: #740

This PR refactors how we fetch repositories with custom properties.

Changes

  • Replace manual endpoint construction (github.request.endpoint) with the built-in github.repos.getCustomPropertiesValues
  • Standardize pagination to 100 items per page to match other API calls
  • Improve error handling
  • Simplify nested loops and improve code readability

Testing Done

Ensured all unit tests pass.

@HagegeR
Copy link

HagegeR commented Jan 20, 2025

FYI:

const existingRepoResults = await github.paginate('GET /installation/repositories')

also does not leverage the 100 results per page

@PendaGTP PendaGTP force-pushed the refactor/improve-get-repositories-by-custom-property branch from 5982a7a to d15fe6d Compare January 20, 2025 09:40
@PendaGTP
Copy link
Contributor Author

@HagegeR

I've also noticed there are other GH API calls in our codebase that don't use the per_page parameter. While this PR focuses specifically on improving how we fetch repositories with custom properties and making that process more readable, I've also included the per_page parameter as an initial step toward addressing issue #740.

I suggest creating a separate PR to comprehensively refactor all GH API calls for consistency and performance.

@PendaGTP PendaGTP force-pushed the refactor/improve-get-repositories-by-custom-property branch from d15fe6d to a178f47 Compare January 20, 2025 13:31
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