-
-
Notifications
You must be signed in to change notification settings - Fork 304
ci: improve line break check ci to change file automatically #1977
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
base: main
Are you sure you want to change the base?
Conversation
SamTheKorean
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ci에 개행 추가 있으면 정말 좋을 것 같네요! 기여감사드립니다!
There was a problem hiding this 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 수동 재실행을 한번 해줘야 할까요?
| # .github 디렉토리 하위 파일 제외하고 대상 파일로 지정 | ||
| files=$(git diff --name-only "$merge_base" ${{ github.event.pull_request.head.sha }} -- . ':(exclude).github/**' | tr -d '"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # .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을 붙이면 해결되는 문제는 아닐까요?
There was a problem hiding this comment.
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 라벨을 붙이면 이 부분이 해결되지 않을 것 같은데, 부가 설명 가능하실까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음 부분 덕분에 #1970 에서도 문제가 없었어요.
leetcode-study/.github/workflows/integration.yaml
Lines 109 to 111 in be10c6c
| # 파일명 규칙 체크 - 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 내 파일의 변경을 고의 또는 실수로 포함하는 경우 기존과 같이 실패 처리하는 것이 더 안전하다는 생각도 드네요.
|
@khg0712 안녕하세요! 병합전에 해당 변경 관련하여 운영진분들과 함께 논의해보았습니다!. 저희 생각에는 이러한 워크플로우의 변경 참여자들의 경험에 어떤 영향을 주는지에 대해서 고민이 좀 더 필요한 것 같습니다. 저희 생각으로는 단순히 체크 수준이 아니라 참여자를 대신에서 직접 commit을 할 경우, 참여자를 이를 모른 체로 로컬에서 다른 commit을 했을 경우, conflict로 이어질 가능성이 높다고 여겨지는데 혹시 이 부분에 대해서 고려해보셨을까요? |
지나가다 아이디어가 떠올라서 간단하게 공유만 드려요~ 현재처럼 CI가 단순 실패만 하지 않고 파일 끝 개행 누락 시 PR에 코멘트를 남기는 방식으로 개선하면
예를 들어, 아래와 같이 파일의 마지막 줄 번호를 구하고, 해당 줄에 코멘트를 달아주는 방식으로 할 수도 있지 않을까 싶었어요! - 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
...쓰다보니 이 방식을 사용할 때 몇 가지 신경써야하는 부분이 있더라고요..!
저는 아이디어만 제공하고 가보겠습니다..! 제가 잘못 이해한 부분이 있거나, 궁금한 부분이 있으면 코멘트로 남겨주세요! 화이팅입니다~! 💪 |
|
제가 생각했던 기존 PR 워크 플로우의 불편한 부분은 다음과 같은 흐름으로 느껴졌습니다.
만약 로컬 변경과 리모트 소스의 차이로 인해 컨플릭이 나는 것이 걱정되신다면 ci에서는 파일 컨벤션 검사만 하고 제 요점은 참여자들의 인지 부하를 줄이고 자동화가능한 영역은 자동화하면 좋을 것 같다 였습니다. |
|
@khg0712 자동 개행 추가가 참여자 편의에 도움이 된다는 점 충분히 공감합니다. 관련 문제 인지하시고 PR 작성해주셔서 감사드립니다! |
line break ci가 자동으로 파일에 개행 문자를 추가하고 커밋하도록 변경합니다.
테스트 레포를 만들어서 해당 변경이 성공한 사례 남깁니다.