-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add comment polling mechanism and improve type safety #77
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
base: main
Are you sure you want to change the base?
Conversation
…o const for better clarity and type safety fix(WorkerInstances.ts): simplify global access by removing unnecessary type casting
…all for future use fix(gh-bugcop.tsx): remove unnecessary type assertion in saveTask function call feat(index.ts): implement comment polling mechanism to check for recent comments every 5 seconds feat(index.ts): add caching for comment timestamps to avoid duplicate processing feat(index.ts): create mock webhook events for new and updated comments detected during polling
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comment polling mechanism for GitHub repositories and improves TypeScript type safety. The changes enable automatic detection of new and updated comments through periodic polling, creating mock webhook events for processing, while also enhancing type declarations throughout the codebase.
- Implement comment polling every 5 seconds with caching to avoid duplicate processing
- Enable TypeScript noImplicitAny option and refactor type declarations
- Create mock webhook events for newly detected comments to integrate with existing processing logic
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/WorkerInstances.ts | Improved type declarations by replacing any types with proper type assertions |
run/index.ts | Added comment polling mechanism with caching and mock webhook event generation |
run/gh-bugcop/gh-bugcop.tsx | Removed unnecessary type assertion in saveTask function call |
run/easylabel.tsx | Added commented-out function call for future use |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
// @ts-ignore TODO fix type | ||
tap((e) => console.log("mocked-webhook-event", e), { | ||
issue_comment: { | ||
action: "created", | ||
issue: { ...issue } satisfies WebhookEventMap["issue_comment"]["issue"], | ||
comment: comment satisfies WebhookEventMap["issue_comment"]["comment"], | ||
repository: { | ||
owner: { login: owner }, | ||
name: repo, | ||
full_name: `${owner}/${repo}`, | ||
} satisfies WebhookEventMap["issue_comment"]["repository"], | ||
sender: comment.user! satisfies WebhookEventMap["issue_comment"]["sender"], | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using @ts-ignore defeats the purpose of enabling noImplicitAny. The TODO comment indicates this should be properly typed. Consider creating a proper type assertion or helper function to construct the webhook event with correct typing."
Copilot uses AI. Check for mistakes.
// @ts-ignore TODO fix type | ||
await this.handleWebhookEvent( | ||
tap(console.debug, { | ||
issue_comment: { | ||
action: "edited", | ||
issue, | ||
comment, | ||
repository: { | ||
owner: { login: owner }, | ||
name: repo, | ||
full_name: `${owner}/${repo}`, | ||
}, | ||
sender: comment.user!, | ||
changes: { | ||
body: { | ||
from: "previous content", // We don't have the old content, but the webhook handler doesn't use it | ||
}, | ||
}, | ||
}, | ||
} satisfies WebhookEventMap), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another @ts-ignore usage that conflicts with the noImplicitAny goal. This duplicated pattern suggests creating a reusable helper function to construct mock webhook events with proper typing would be beneficial."
Copilot uses AI. Check for mistakes.
sender: comment.user!, | ||
changes: { | ||
body: { | ||
from: "previous content", // We don't have the old content, but the webhook handler doesn't use it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded placeholder string creates potential confusion. Consider using a more explicit placeholder like 'UNKNOWN_PREVIOUS_CONTENT' or define a constant to make the intent clearer."
Copilot uses AI. Check for mistakes.
Summary
Changes
Test plan
🤖 Generated with Claude Code