Conversation
|
|
||
| const cache: { [key: string]: ITeam[] } = {}; | ||
| const hasAllRepos = (await Promise.all(this._githubRepositories.map(async (repo) => { | ||
| const key = `${repo.remote.owner}/${repo.remote.repositoryName}.json`; |
There was a problem hiding this comment.
There could be name collisions with enterprise instances.
There was a problem hiding this comment.
I think we could only get a collision if there are two unrelated enterprises with the same owner + repo name combination. Is that correct?
There was a problem hiding this comment.
Because the cache key makes no distinction between repo origins, yes, two unrelated enterprises might collide. But more importantly and likely would be a collision between github.com repos and any enterprise.
Example:
github.com/microsoft/vscodegithub.contoso.net/microsoft/vscode
While it may be unlikely that these examples exist and represent distinct teams, they are indeed two different sources of truth for information when querying/caching on the context microsoft/vscode
There was a problem hiding this comment.
Got it, thanks. I'll update the key in another PR.
|
Moving this out, I hope for the last time. The reason I haven't merged it yet is that we're waiting to see if GitHub can provide a way to get team reviewers without adding the |
alexr00
force-pushed
the
alexr00/issue1126
branch
from
3acde33 to
649a2b7
Compare
4 participants
Fixes #1126