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

feat: issue opened event #52

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

Conversation

rojen11
Copy link
Member

@rojen11 rojen11 commented Jan 20, 2023

No description provided.

@rojen11 rojen11 requested a review from ashiishme January 20, 2023 09:57
Comment on lines 3 to 7
sender: { [key: string]: unknown };
repository: { [key: string]: unknown };
organization: { [key: string]: unknown };
installation: { [key: string]: unknown };
[key: string]: unknown;
Copy link
Member

Choose a reason for hiding this comment

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

So, we don't know the data type of these properties? As the value is set to unknown.

Copy link
Member

Choose a reason for hiding this comment

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

This type can be constructed using Record utility type.

so, [key: string]: unknown => Record<string, unknown>

Copy link
Member Author

Choose a reason for hiding this comment

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

image
Github has labelled the type as object so I used unknown.

Copy link
Member Author

Choose a reason for hiding this comment

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

This type can be constructed using Record utility type.

so, [key: string]: unknown => Record<string, unknown>

Done.

Comment on lines 11 to 12
body: string | null;
title: string;
Copy link
Member

Choose a reason for hiding this comment

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

If we just need body and title it's okay to define only them although the payload has other props.

Copy link
Member Author

Choose a reason for hiding this comment

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

image
I got this error when I removed the extra props.

// check if the event is from registered repository
if (
!askBuddieBot.isValidRepository(
(payload.repository?.name as string) ?? ''
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work?

Suggested change
(payload.repository?.name as string) ?? ''
payload.repository?.name ?? ''

Copy link
Member Author

Choose a reason for hiding this comment

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

image
i get this error without typing it as string.

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