Skip to content
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

통합 워크플로우 #29

Merged
merged 3 commits into from
Oct 23, 2024
Merged

통합 워크플로우 #29

merged 3 commits into from
Oct 23, 2024

Conversation

DaleSeo
Copy link
Contributor

@DaleSeo DaleSeo commented Oct 23, 2024

PR에 새로운 commit을 push 할 때 마다 자동으로 코드 포멧, 린트, 테스트 (커버리지), 빌드를 실행해주는 워크플로우를 추가합니다. 이 중에 하나라도 실패하게 되면 PR을 머지할 수 없게 됩니다. 이러한 자동화를 통해 코드 베이스의 최소한의 품질을 확보하며 불필요한 리뷰 코멘트를 방지합니다.

  • GitHub Actions 로그

Shot 2024-10-22 at 21 32 36@2x

  • PR 하단 체크

Shot 2024-10-22 at 21 45 40@2x

@DaleSeo DaleSeo marked this pull request as ready for review October 23, 2024 01:37
@DaleSeo DaleSeo requested a review from a team as a code owner October 23, 2024 01:37
Comment on lines +9 to +15
beforeAll(() => {
vi.spyOn(console, "error").mockImplementation(() => {});
});

afterAll(() => {
vi.mocked(console.error).mockRestore();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

테스트 결과에 어지럽게 에러 로그가 출력되지 않도록 하기 위함입니다.

  • before

Shot 2024-10-22 at 21 38 32@2x

  • after

Shot 2024-10-22 at 21 41 14@2x

@DaleSeo DaleSeo linked an issue Oct 23, 2024 that may be closed by this pull request
11 tasks
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.

에러 결과 나오는 거 은근 dx를 떨어뜨려서 신경쓰였는데 감사합니다!

@sounmind
Copy link
Member

sounmind commented Oct 23, 2024

각 동작이 waterfall로 실행되는 걸까요? 만약 그렇다면 그것을 혹시 의도하신 걸까요?
build, test, lint가 꼭 waterfall로 실행되어야 할 필요는 없다고 생각했는데요.
또 build, test, lint 다 별개의 결과로 보여준다면, 개발자가 순차적으로 하나하나 고쳐 나가면서 다음 단계를 확인하지 않아도 되기 때문에 더 좋은 경험을 할 것 같아 질문드립니다.

@DaleSeo
Copy link
Contributor Author

DaleSeo commented Oct 23, 2024

각 동작이 waterfall로 실행되는 걸까요? 만약 그렇다면 그것을 혹시 의도하신 걸까요? build, test, lint가 꼭 waterfall로 실행되어야 할 필요는 없다고 생각했는데요. 또 build, test, lint 다 별개의 결과로 보여준다면, 개발자가 순차적으로 하나하나 고쳐 나가면서 다음 단계를 확인하지 않아도 되기 때문에 더 좋은 경험을 할 것 같아 질문드립니다.

@sounmind 좋은 질문 감사합니다! waterfall 이라고 보실 수도 있지만 short circuit이라고 보실 수도 있을 것 같아요. 예를 들어, 코드 포멧팅 조차 안 되어 있는 코드를 상대로 굳이 lint, coverage나 build를 돌리는 것은 제한된 GitHub Actions의 billable time만 낭비할 수 있으니까요.

이미 프로젝트에 husky와 pre commit 설정이 되었으니 대부분의 문제는 개발자가 로컬에서 commit을 할 때 잡힐 것입니다. 통합 워크플로우는 주로 개발자가 미처 로컬에서 발견하지 못했거나, 커밋을 할 때 -n 옵션 등을 사용하여 우회한 문제가 있는 commit을 잡아내기 위한 목적이 큽니다. 말씀하신 것처럼 통합 워크플로우의 여러 단계가 실패하는 PR을 우선 올려놓고 하나씩 고쳐 나가는 것은 매우 비효율적인 개발 프로세스라는 생각이 듭니다.

저는 처음에는 워크플로우를 단순하게 가져가다가 오래 걸리는 단계가 생기면 별도 작업으로 빼는 것을 선호하는 편입니다. 느린 단계는 병렬로 돌리면 개발자에게 더 빠른 피드백을 줄 수 있기 때문입니다. 하지만 우리 프로젝트의 경우, Bun이 말도 안 되게 빨라서 모든 단계를 실행하는데 20초도 안 걸리는 상황입니다. 우선 하나의 작업으로 시작하고 나중에 프로젝트가 커져서 빌드 시간이 증가한다면, build만 따로 돌리도록 워크플로우를 수정하면 어떨까요?

Shot 2024-10-23 at 06 15 46@2x

@sounmind
Copy link
Member

와, 상세하고 친절한 설명 감사합니다. 의도를 충분히 파악했습니다 ☺️
Github Action 자원도 제한적이고, bun도 충분히 빠르고, 추후 시간이 많이 걸리는 과정을 빼는 결정도 동의합니다.

Copy link
Member

@sounmind sounmind left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@DaleSeo DaleSeo merged commit feaaa8f into main Oct 23, 2024
1 check passed
@DaleSeo DaleSeo deleted the ci-integration branch October 23, 2024 15:42
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.

CI/CD 구축
3 participants