Skip to content

Slightly better typing and doc comment for query clickhouse #6607

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

clee2000
Copy link
Contributor

@clee2000 clee2000 commented May 8, 2025

Move doc comment so that it is compatible with vscode hover

Also add example for output

More specific typing for the output of queryclickhouse, this results in a bunch of (mostly type related changes in other places)

not sure if this is a good idea

Copy link

vercel bot commented May 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
torchci ✅ Ready (Inspect) Visit Preview May 8, 2025 7:01pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 8, 2025
@clee2000 clee2000 marked this pull request as ready for review May 8, 2025 23:27
@clee2000 clee2000 requested a review from a team May 8, 2025 23:27
}

acc[row.pr_num].push(row.merge_commit_sha);
acc.get(row.pr_num)!.push(row.merge_commit_sha);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

idk what happened to the typing here, it starts out as a map, but then acc is {key: string} and the return value is also a map

so im just making it always a map

@@ -77,7 +77,7 @@ export async function fetchFlakyTestsAcrossFileReruns(
if (!workflowJobMap.has(curr.job_id)) {
return accum;
}
const job_info = workflowJobMap.get(curr.job_id);
const job_info = workflowJobMap.get(curr.job_id)!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't know why suddenly need ! but typechecker complains about undefined later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants