Skip to content

✨ 더 공평한 리뷰어 선정 #398

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 8 commits into
base: develop
Choose a base branch
from
Open

Conversation

goranikin
Copy link
Collaborator

close #366

작업 내용

최근 PR 목록을 가져온 뒤, 각각의 목록의 Reviewer들을 counts해 가장 적은 수의 사람을 선택합니다.

2명 이상일 경우 random으로 지정해요!

@goranikin goranikin self-assigned this May 3, 2025
@goranikin goranikin changed the title 더 공평한 리뷰어 선정 ✨ 더 공평한 리뷰어 선정 May 3, 2025
@goranikin goranikin closed this May 3, 2025
@goranikin goranikin reopened this May 3, 2025
Copy link

github-actions bot commented May 3, 2025

@yeolyi님 2025. 5. 6.까지 리뷰 부탁드립니다 🎉

@github-actions github-actions bot requested a review from yeolyi May 3, 2025 11:49
@@ -11,17 +11,49 @@ jobs:
random-reviwer:
runs-on: ubuntu-latest
steps:
- id: random_reviwer
Copy link
Collaborator

Choose a reason for hiding this comment

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

앗 ㅋㅋㅋㅋㅋ 👍👍

Comment on lines +37 to +48
for (const pr of recentPRs.data) {
const reviewers = await github.rest.pulls.listRequestedReviewers({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pr.number
});
for (const reviewer of reviewers.data.users) {
if (reviewCounts[reviewer.login] !== undefined) {
reviewCounts[reviewer.login]++;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

루프 안에 await이 있는데 Promise.all을 활용해보는건 어떤가요?

Comment on lines +32 to +35
const reviewCounts = {};
candidateList.forEach(reviewer => {
reviewCounts[reviewer] = 0;
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래에서는 for of를 사용하고 여기서는 forEach를 사용했는데 의도가 있을까요? 없다면 통일해도 좋을 것 같습니다 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

아래쪽에서 map이랑 filter를 쓰는걸 보면 함수형 프로그래밍 느낌이 좀 나는데 여기도 reduce로 바꿔볼 수는 있겠네요. 상관은 없습니다 ㅎㅎ

Comment on lines +23 to +30
const recentPRs = await github.rest.pulls.list({
owner: context.repo.owner,
repo: context.repo.repo,
state: 'all',
sort: 'updated',
direction: 'desc',
per_page: 10
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

사소한데 도움이 될까해서 남기자면...
이건 개발자별로 취향일 수 있지만 저는 변수 정의 위치랑 사용 위치가 가까운걸 좋아하는데요,
recentPRs는 저 아래 for of에서 처음 사용되니 변수 위치를 좀 내려도 좋을 것 같습니다.

Copy link
Collaborator

@yeolyi yeolyi left a comment

Choose a reason for hiding this comment

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

그나저나 github action에서 top-level await가 되는지 모르겠네요. 나중에 테스트해보고 안되면 대응하면 되겠습니다~

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