Skip to content

Conversation

@khg0712
Copy link
Contributor

@khg0712 khg0712 commented Nov 8, 2025

line break ci가 자동으로 파일에 개행 문자를 추가하고 커밋하도록 변경합니다.

테스트 레포를 만들어서 해당 변경이 성공한 사례 남깁니다.

  • .github/ 하위 디렉토리가 파일 명 컨벤션 ci에 걸려서 이것을 제외하도록 변경하는 수정도 추가합니다
image image

Copy link
Contributor

@SamTheKorean SamTheKorean left a comment

Choose a reason for hiding this comment

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

ci에 개행 추가 있으면 정말 좋을 것 같네요! 기여감사드립니다!

Copy link
Contributor

@yhkee0404 yhkee0404 left a comment

Choose a reason for hiding this comment

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

기존에는 줄바꿈 문제로 Check 실패를 발생시켰는데 자동 해결 후에도 아직 그러는 것 같아요: https://github.com/khg0712/ci-test/actions/runs/19196438420/job/54878083183
exit 1 부분까지 삭제하면 이제 줄바꿈 문제는 Check 실패가 없는 거겠죠?
아니면 누군가 Check 수동 재실행을 한번 해줘야 할까요?

Comment on lines +137 to +138
# .github 디렉토리 하위 파일 제외하고 대상 파일로 지정
files=$(git diff --name-only "$merge_base" ${{ github.event.pull_request.head.sha }} -- . ':(exclude).github/**' | tr -d '"')
Copy link
Contributor

@yhkee0404 yhkee0404 Nov 9, 2025

Choose a reason for hiding this comment

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

Suggested change
# .github 디렉토리 하위 파일 제외하고 대상 파일로 지정
files=$(git diff --name-only "$merge_base" ${{ github.event.pull_request.head.sha }} -- . ':(exclude).github/**' | tr -d '"')
files=$(git diff --name-only $merge_base ${{ github.event.pull_request.head.sha }} | tr -d '"')

이 부분을 고치는 대신 maintenance label을 붙이면 해결되는 문제는 아닐까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분을 고치는 대신 maintenance label을 붙이면 해결되는 문제는 아닐까요?

이 부분이 잘 이해가 안됐어요. .github 하위 파일들은 알고리즘 관련 파일이 아니기 때문에 {github 닉네임}.xxx 파일 컨벤션을 제외하도록 했습니다. maintenance 라벨을 붙이면 이 부분이 해결되지 않을 것 같은데, 부가 설명 가능하실까요?

Copy link
Contributor

@yhkee0404 yhkee0404 Nov 9, 2025

Choose a reason for hiding this comment

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

다음 부분 덕분에 #1970 에서도 문제가 없었어요.

# 파일명 규칙 체크 - maintenance 라벨이 없는 경우에만 실행
- name: Check filename rules
if: ${{ steps.pr-labels.outputs.has_maintenance != 'true' }}

.github 디렉토리 외에도 일일이 예외 처리하는 Whitelist를 관리하는 것보다 검사 대상 디렉토리만 지정하는 Blacklist 방식도 좋을 것 같고요. maintenance label은 사전에 정적으로 정의한 디렉토리 수준이 아니라 Pull Request 단위로 검사 여부를 동적으로 결정할 수 있어서 좋은 것 같아요.
또한 maintenance label이 없는 일반 풀이 제출 Pull Request가 만일 지금처럼 .github 내 파일의 변경을 고의 또는 실수로 포함하는 경우 기존과 같이 실패 처리하는 것이 더 안전하다는 생각도 드네요.

@SamTheKorean
Copy link
Contributor

SamTheKorean commented Nov 9, 2025

@khg0712 안녕하세요! 병합전에 해당 변경 관련하여 운영진분들과 함께 논의해보았습니다!. 저희 생각에는 이러한 워크플로우의 변경 참여자들의 경험에 어떤 영향을 주는지에 대해서 고민이 좀 더 필요한 것 같습니다. 저희 생각으로는 단순히 체크 수준이 아니라 참여자를 대신에서 직접 commit을 할 경우, 참여자를 이를 모른 체로 로컬에서 다른 commit을 했을 경우, conflict로 이어질 가능성이 높다고 여겨지는데 혹시 이 부분에 대해서 고려해보셨을까요?

@unpo88
Copy link
Contributor

unpo88 commented Nov 10, 2025

@khg0712 안녕하세요! 병합전에 해당 변경 관련하여 운영진분들과 함께 논의해보았습니다!. 저희 생각에는 이러한 워크플로우의 변경 참여자들의 경험에 어떤 영향을 주는지에 대해서 고민이 좀 더 필요한 것 같습니다. 저희 생각으로는 단순히 체크 수준이 아니라 참여자를 대신에서 직접 commit을 할 경우, 참여자를 이를 모른 체로 로컬에서 다른 commit을 했을 경우, conflict로 이어질 가능성이 높다고 여겨지는데 혹시 이 부분에 대해서 고려해보셨을까요?

지나가다 아이디어가 떠올라서 간단하게 공유만 드려요~

현재처럼 CI가 단순 실패만 하지 않고 파일 끝 개행 누락 시 PR에 코멘트를 남기는 방식으로 개선하면
Conflict도 방지되고, 개행이 있을 경우 알림을 주어 참여자 경험을 조금 더 부드럽게 만들 수 있지 않을까 합니다 🤔

예를 들어, 아래와 같이 파일의 마지막 줄 번호를 구하고, 해당 줄에 코멘트를 달아주는 방식으로 할 수도 있지 않을까 싶었어요!
(실제 정확하게 동작하는 코드인지 확인을 하지 않아서 수도 코드로 봐주세요!)

      - name: Check for missing end line breaks
        env:
           ...
        run: |
           ...
          echo "## 줄바꿈 누락 파일" >> $GITHUB_STEP_SUMMARY

          for file in $files; do
            if [ -f "$file" ]; then
              if [ -s "$file" ] && [ "$(tail -c 1 "$file" | wc -l)" -eq 0 ]; then
                echo "❌ 줄바꿈 누락: $file"
                echo "- $file" >> $GITHUB_STEP_SUMMARY
                success=false

               # 마지막 줄 번호 구하기
               last_line=$(wc -l < "$file")
                [ "$last_line" -eq 0 ] && last_line=1

                # 코멘트 payload 구성
                payload=$(jq -n \
                  --arg body "❌ 파일 끝에 줄바꿈이 없습니다." \
                  --arg commit_id "${{ github.event.pull_request.head.sha }}"
                  --arg path "$file" \
                  --argjson line "$last_line" \
                  '{body: $body, commit_id: $commit_id, path: $path, line: $line, side: "RIGHT"}')

                 curl --silent --fail --show-error \
                  -H "Accept: application/vnd.github+json" \
                  -H "Authorization: Bearer $GITHUB_TOKEN" \
                  -H "X-GitHub-Api-Version: 2022-11-28" \
                  -d "$payload" \
                  "https://api.github.com/repos/${{ github.repository }}/pulls/${PR_NUMBER}/comments" \
                  || echo "⚠️ GitHub 코멘트 등록 실패 (권한 문제 또는 잘못된 commit_id)"

              else
       ...

쓰다보니 이 방식을 사용할 때 몇 가지 신경써야하는 부분이 있더라고요..!

  • API 호출이 빈번하면 GitHub API rate limit에 걸릴 수 있다
  • 매번 workflow가 돌 때마다 같은 파일에 같은 코멘트가 계속 달릴 수 있습니다 😅
  • PR 코멘트를 쓰려면 workflow 권한에 write 권한이 필요할 것 같아요.

저는 아이디어만 제공하고 가보겠습니다..! 제가 잘못 이해한 부분이 있거나, 궁금한 부분이 있으면 코멘트로 남겨주세요! 화이팅입니다~! 💪

@khg0712
Copy link
Contributor Author

khg0712 commented Nov 10, 2025

제가 생각했던 기존 PR 워크 플로우의 불편한 부분은 다음과 같은 흐름으로 느껴졌습니다.

  1. WIKI에 참여자가 알아야 하는 정보가 많은데 파일 개행관련 규칙은 참여자가 알지 않더라도 시스템적으로 자동화하면 더 좋겠다.
  2. 단순히 코멘트나 Ci 실패로 알려줘도 참여자가 인지할 수는 있겠으나 사용자가 해당 알림에 대해 팔로업을 해야 하고 비동기 소통이 기반인 커뮤니티에서 이런 워크플로우는 흐름을 더 느리게 만들 수 있어보인다.
  3. 파일 N개에 대해서 수동으로 파일 개행을 추가하는 것이 생각보다 불편한데 해당 작업이 복잡한 파일변경이 아니기 때문에 자동화하면 편리하겠다.

만약 로컬 변경과 리모트 소스의 차이로 인해 컨플릭이 나는 것이 걱정되신다면 ci에서는 파일 컨벤션 검사만 하고
이런 자동화는 pre-commit hook이나 pre-push 훅에서 처리하면 해당 문제는 없어질 것 같습니다.

제 요점은 참여자들의 인지 부하를 줄이고 자동화가능한 영역은 자동화하면 좋을 것 같다 였습니다.

@SamTheKorean
Copy link
Contributor

SamTheKorean commented Nov 10, 2025

@khg0712 자동 개행 추가가 참여자 편의에 도움이 된다는 점 충분히 공감합니다. 관련 문제 인지하시고 PR 작성해주셔서 감사드립니다!
다만, CI가 직접 커밋이나 푸시를 수행하는 구조는 지양하는 것이 좋을 것 같습니다!.
휴고님이 주신 대안처럼 pre-commit hook이나 pre-push 단계로 옮기는 방향이 더 적절해 보입니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Solving

Development

Successfully merging this pull request may close these issues.

4 participants