Skip to content

feat [mumu]: issue_comment 핸들러 추가 및 코멘트 핸들러 통합#17

Open
wuzoo wants to merge 3 commits intomainfrom
feat/handle-issue-comment
Open

feat [mumu]: issue_comment 핸들러 추가 및 코멘트 핸들러 통합#17
wuzoo wants to merge 3 commits intomainfrom
feat/handle-issue-comment

Conversation

@wuzoo
Copy link
Member

@wuzoo wuzoo commented Mar 19, 2026

작업 내용

  • PR 대화 탭(Conversation) 코멘트에 대한 issue_comment 웹훅 처리 추가
  • pull_request_review_comment / issue_comment를 Comment 유니온으로 받아 handleComment 하나에서 issue in payload로 분기하도록 통합
  • 공통 로직(Redis 스레드 조회, 메시지 생성, 슬랙 전송)을 handleComment 내부로 모음

Made with Cursor

@wuzoo wuzoo self-assigned this Mar 19, 2026
action: z.enum(["created", "edited", "deleted"]),
issue: z.object({
number: z.number(),
pull_request: z.object({}).passthrough().optional(),
Copy link
Member

Choose a reason for hiding this comment

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

제 ide에서 이렇게 뜨네용

Image

Copy link
Member

Choose a reason for hiding this comment

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

'passthrough' 이 부분입니다.

pull_request: z.object({}).passthrough().optional(),

Choose a reason for hiding this comment

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

찾아보니 zod v4에서 deprecated 되었다고 합니다..?!
https://zod.dev/v4/changelog
image

Copy link
Member Author

Choose a reason for hiding this comment

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

호호.. 커서가 그것까진 몰랐나봐요 ㅎ.. 수정하겠습니다

Copy link
Member Author

@wuzoo wuzoo Mar 21, 2026

Choose a reason for hiding this comment

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

Copy link
Member

@jogpfls jogpfls left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 !! 코드 너무 어렵네요... 코드리뷰하면서 많이 배워갑니다 !!! 😊

Comment on lines 53 to +59
try {
await slackNotifier.createThreadReply(thread.threadTs, text);
} catch {
console.error(`${cacheKey}/${thread.channel}: review comment 슬랙 스레드 답변 전송 실패`);
console.error(`${cacheKey}/${thread.channel}: 슬랙 스레드 답변 전송 실패`);
}

return JSON.stringify({ success: true, message: "Review comment processed successfully" });
return JSON.stringify({ success: true, message: "Comment processed successfully" });
Copy link
Member

Choose a reason for hiding this comment

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

createThreadReply이 실패했을 때 catch 블록이 에러를 잡으면 console.error만 찍고 끝난 다음에 success: true가 실행되고 있어서 slack 전송이 실패해도 성공으로 리턴하는 구조인 것 같은데 의도하신 구조이신가요 ?!

Copy link
Member Author

Choose a reason for hiding this comment

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

오 아니에요! 요건 사실 저번에도 비슷한 리뷰를 받았었는데요. 어느 계층에서 로그를 남길것이냐를 고민하다가 저런 식으로 작성되었는데,
여기는 comment 핸들러 안에서의 메인 액션인 것 같아서 return문 하나 더 찍어두도록 하겠습니다. 나머지는 추후 리팩토링하겠습니다..!

Copy link
Member Author

Choose a reason for hiding this comment

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

const preview = truncateBody(payload.comment.body);
const safePreview = escapeSlackLinkText(preview);

return [`> *${payload.comment.user.login}*`, `> <${payload.comment.html_url}|${safePreview}>`].join("\n");
Copy link
Member

@jogpfls jogpfls Mar 20, 2026

Choose a reason for hiding this comment

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

issueComment랑 reviewComment가 구분이 따로 안되고 있는 것 같은데 일부러 이렇게 구현하신걸까요 ? 아니시라면 앞에 [issueComment], [reviewComment] 이런식으로 붙여서 구분하는 건 어떻게 생각하시나요...? Just 개인적인 의견입니당....

Copy link
Member Author

Choose a reason for hiding this comment

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

네네 사실 issue 코멘트랑 review 코멘트를 구분할 필요성을 못느꼈어서 따로 처리해두지는 않았습니다.

혜린님이 생각하신 구분되어야 할 이유가 혹시 무엇일까요 ?!

@wuzoo wuzoo requested review from ExceptAnyone and jogpfls March 21, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants