fix: Re-enable creation of the new repositories #833
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the repository-fetch logic to include repos defined in the config but missing from the API response.
- Introduces tracking of processed repos to avoid duplicates
- Splits processing into existing and missing repos batches
- Concatenates and filters both result sets before returning
|
This change still requires |
|
Hi @klutchell. Yes, it requires safe-settings/lib/plugins/repository.js Line 121 in 3e5d4fe Thanks |
There was a problem hiding this comment.
Pull Request Overview
A fix to ensure newly configured repositories that aren’t returned by the installation API still get processed.
- Split repository handling into “existing” (via GitHub paginate) and “missing” (from configuration) batches
- Track which repos have been processed to avoid duplicates
- Concatenate results from both batches and filter out nulls
Comments suppressed due to low confidence (1)
lib/settings.js:508
- [nitpick] Rename
repoInConfigsto something likeconfiguredReposorreposFromConfigto more clearly express its purpose.
const repoInConfigs = Object.values(this.repoConfigs)
| const results = [] | ||
|
|
There was a problem hiding this comment.
The results array is declared but never populated; remove it and simplify the return to directly combine existingRepoResults and missingRepoResults.
| const results = [] |
Copilot uses AI. Check for mistakes.
| const existingRepoResults = await github.paginate('GET /installation/repositories') | ||
| .then(repositories => { | ||
| return Promise.all(repositories.map(repository => { | ||
| if (this.isRestricted(repository.name)) { | ||
| return null | ||
| } | ||
| const { owner, name } = repository | ||
| processedRepos.add(`${owner.login}/${name}`) | ||
| return this.updateRepos({ owner: owner.login, repo: name }) | ||
| })) | ||
| }) |
There was a problem hiding this comment.
Avoid mixing await with .then; instead, do const repositories = await github.paginate(...) and then const existingRepoResults = await Promise.all(...) for clarity.
Copilot uses AI. Check for mistakes.
Hi all,
This PR fixes issue safe-settings/issues/780.
The code that returns the list of all repositories does not include one that does not exist.
Cheers! 👋