Skip to content

ci: add assign-reviewer job #529

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

Merged
merged 7 commits into from
Nov 10, 2024
Merged

ci: add assign-reviewer job #529

merged 7 commits into from
Nov 10, 2024

Conversation

bky373
Copy link
Contributor

@bky373 bky373 commented Oct 13, 2024

배경

image

작업 내용

  • GitHub CLI 를 사용하여 PR 리뷰어를 자동 할당하는 automation job 을 추가합니다.
  • 리뷰어 조회 조건
      - Closed PR 은 제외함 (실수로 올렸다 닫는 경우가 종종 있으므로) -> opened, merged PR 만 조회 (하단 코멘트 참고)
      - 찾은 PR 중 현재 PR에 가장 근접한 PR의 작성자여야 함 (created 로 역순 정렬)
      - 현재 PR과 다른 이름의 작성자여야 함
      - 현재 PR 보다 이전에 생성된 PR의 작성자여야 함
previous_pr_author=$(gh pr list --repo $current_repo \
            --search "-is:closed sort:created-desc -author:$current_pr_author" \
            --limit 3 --json number,author \
            --jq 'map(select(.number < '$current_pr_num'))[0].author.login')
  • 그외 .gitignore 업데이트

테스트

image

한계

  • 현재 PR처럼 문제풀이와 상관없는 PR이 올라올 경우에도, 마지막 PR 작성자를 리뷰어로 할당받게 됨. (지금은 merge가 안됐기 때문에 적용되진 않지만)
  • 새로운 기수가 처음 등록하는 PR 에서는 직전 기수의 마지막 PR 작성자를 리뷰어로 할당받게 됨.

위 두 문제는 식별가능한 label 을 미리 설정하는 것으로 해결할 수 있을 것 같지만 그렇게 하기엔 손이 많이 가서 우선 보류합니다..

- PR 리뷰어를 자동 할당하는 automation job 을 추가합니다.
- 리뷰어 찾기 조건
  - 현재 PR 보다 이전에 생성된 PR 이어야 함
  - Closed PR 은 제외해야 함 (실수로 올렸다 닫는 경우가 종종 있으므로)
  - 찾은 PR 중 현재 PR과 가장 근접한 PR의 작성자여야 함
  - 현재 PR과 다른 이름의 작성자여야 함
@bky373 bky373 requested a review from a team as a code owner October 13, 2024 03:51
@bky373 bky373 requested a review from DaleSeo October 13, 2024 03:55
# 최근 3개의 PR 중 현재 PR 작성자와 다른 작성자 찾기 ()
previous_pr_author=$(gh pr list --repo $current_repo \
--search "number:<$current_pr_num -is:closed sort:created-desc -author:$current_pr_author" \
--limit 3 --json number,author \
Copy link
Contributor Author

@bky373 bky373 Oct 13, 2024

Choose a reason for hiding this comment

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

여기서 number 는 조회하지 않아도 되겠네요 이것저것 테스트해보다 넣었는데 ㅎㅎ 빼겠습니다 -> 7a10e64 완료

Copy link
Contributor Author

Choose a reason for hiding this comment

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

search 옵션에서 pr number 를 따로 사용할 수 없어서, c8e1b5f 에서 pr num 비교하는 부분을 변경하였습니다.

Copy link
Member

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

  • Closed PR 은 제외함 (실수로 올렸다 닫는 경우가 종종 있으므로) -> opened, merged PR 만 조회

@bky373 지금 문제의 핵심이 멤버들이 실수로 전전 PR 작성자가 리뷰어로 추가하는 거거든요? 전 PR이 너무 빨리 머지되서요. 이 문제를 방지하려면 Closed PR도 포함해야 되지 않을까요?

한계

이거는 큰 문제가 될 것 같지는 않아요. 여전히 PR 작성자가 리뷰어를 직접 바꿀 수 있으니까요.

@bky373
Copy link
Contributor Author

bky373 commented Oct 18, 2024

  • Closed PR 은 제외함 (실수로 올렸다 닫는 경우가 종종 있으므로) -> opened, merged PR 만 조회

@bky373 지금 문제의 핵심이 멤버들이 실수로 전전 PR 작성자가 리뷰어로 추가하는 거거든요? 전 PR이 너무 빨리 머지되서요. 이 문제를 방지하려면 Closed PR도 포함해야 되지 않을까요?

네네 Closed PR 포함하는 건 변경 커밋에 포함해두겠습니다~

Copy link
Contributor Author

@bky373 bky373 left a comment

Choose a reason for hiding this comment

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

이런저런 일들로 작업이 늦어졌네요 ㅠㅠ 😢
개인 레포에서는 테스트 마쳤는데 확인 한 번 해주시면 감사하겠습니다! cc @DaleSeo

--search "-is:closed sort:created-desc -author:$current_pr_author" \
--limit 3 --json number,author \
--jq 'map(select(.number < '$current_pr_num'))[0].author.login')
--state all \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

state 기본값"open" 이라 all 로 설정하였습니다
image

--limit 3 --json number,author \
--jq 'map(select(.number < '$current_pr_num'))[0].author.login')
--state all \
--search "created:<${{ github.event.pull_request.created_at }} sort:created-desc -author:${{ github.actor }}" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

현재 PR 이전에 생성된 PR 이어야 하므로, created: 조건을 추가하였습니다.
${{ github.event.created_at }} 는 따로 제공하는 변수가 아니라서 ${{ github.event.pull_request.created_at }} 를 사용했습니다.

--search "created:<${{ github.event.pull_request.created_at }} sort:created-desc -author:${{ github.actor }}" \
--limit 3 \
--json number,author \
--jq "map(select(.number < $current_pr_num))[0].author.login")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$current_pr_num 를 ' 로 감싸면 문자열로 인식하여 '를 제거하였습니다.

.gitignore Outdated
@@ -2,3 +2,4 @@
.vscode/
.DS_Store
.env
**/*.iml
Copy link
Member

Choose a reason for hiding this comment

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

이 건 왜 추가되는 거에요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 제가 인텔리제이 IDE를 사용해서 작업하였는데 인텔리제이 모듈이 자동으로 생성되어 추가했었습니다
요건 크게 의미있지는 않으니 제거하고 머지하겠습니당!

@bky373 bky373 merged commit 675caa0 into DaleStudy:main Nov 10, 2024
1 check passed
bky373 added a commit that referenced this pull request Nov 11, 2024
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