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

ref: Optimize file fetching code #96

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

spalmurray-codecov
Copy link
Collaborator

While looking around in here I noticed we were doing a whole extra fetch to get the metadata for the file when all of the info we need was already present on the page. This PR updates the code to remove that fetch. Additionally, adds support for file coverage when browsing at a specific commit, this was previously not supported.

headers: {
"Accept": "application/json",
},
}).then((response) => response.json());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🫡

if (commit.length === 7) {
const commitLink = document.querySelector(
`[href^="/${groups.owner}/${groups.repo}/tree/${commit}"]`
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the long sha can be found inside a few links on this page, that's what this code extracts.

@@ -232,40 +269,6 @@ async function process(metadata: FileMetadata): Promise<void> {
animateAndAnnotateLines(noVirtLineSelector, annotateLine);
}

async function promptPastReport(metadata: FileMetadata): Promise<void> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There will never be a need for this code anymore. This was called when the metadata for a branch gave the HEAD commit and there was no coverage for that HEAD commit. If you're on a branch now, Codecov will handle this on the API side and give you the last good coverage.

.closest("div")!;
const dropdownButton = editButton.cloneNode(true) as HTMLElement;
const textNode: HTMLElement = dropdownButton.querySelector('[data-component="IconButton"]')!;
// Build the button out of the Raw/copy/download button group
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you're looking at a file at a commit, the old button we were building from does not exist. I adjusted the code to use a button that exists on both types of view (the raw/copy/download group). Additionally, I made the code a bit safer by removing !s

@spalmurray-codecov spalmurray-codecov marked this pull request as ready for review November 19, 2024 17:39
src/content/github/file/main.tsx Outdated Show resolved Hide resolved
src/content/github/file/main.tsx Outdated Show resolved Hide resolved
@spalmurray-codecov spalmurray-codecov merged commit 662b957 into main Nov 19, 2024
1 check passed
@spalmurray-codecov spalmurray-codecov deleted the spalmurray/optimize-file-fetching branch November 19, 2024 19:27
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