Only use the default branch to determine repo-age#3445
Only use the default branch to determine repo-age#3445fregante merged 5 commits intorefined-github:masterfrom
repo-age#3445Conversation
source/features/repo-age.tsx
Outdated
| day: 'numeric' | ||
| }); | ||
|
|
||
| const getCommitCount = async (): Promise<AnyObject> => { |
There was a problem hiding this comment.
No need to extract this into its own function though, especially considering the AnyObject typing. Just merge it back into getFirstCommit
There was a problem hiding this comment.
Besides that everything else is ok?
There was a problem hiding this comment.
I remember why I split it. I was having issues with constants they both are returning the same names.
There was a problem hiding this comment.
I don't know what you're talking about
There was a problem hiding this comment.
That's what happens when I reply half in my sleep. Sorry I misunderstood you I thought you wanted both api requests in one function.
Anyways I'm up now ☕
fregante
changed the title
repo-agerepo-age
|
Thanks for this fix! Negative diffs are the best |
|
|
||
| // Returning undefined will make sure that it is not cached. It will check again for commits on the next load. | ||
| // Reference: https://github.com/fregante/webext-storage-cache/#getter | ||
| if (commitsCount === 0) { |
There was a problem hiding this comment.
I just realized that this was removed. Why? It shouldn't be
There was a problem hiding this comment.
Since until now the element could have not been ready on time so it would be undefined. Now the only case would be on a brand new repo.
There was a problem hiding this comment.
There was a problem hiding this comment.
Now the only case would be on a brand new repo.
Yes. Brand new repos still exist and now I think this would throw in those cases because repository.defaultBranchRef.target is null
There was a problem hiding this comment.
The easiest way is to add isEmptyRepoRoot to excludes
TODO lint <-- search this next week :D
2 participants
Follows #3340 (comment)