-
-
Notifications
You must be signed in to change notification settings - Fork 2
릴리즈 발행시 동작하는 배포 워크플로우 생성 #654
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
842a750 to
c403cdf
Compare
|
@HowToBeAHappyBoy 님, 요거 계속 진척이 없네요? 병목이 될까봐 걱정이 됩니다. 업데이트 좀 부탁드릴께요! |
제가 지금 좀 헷갈려서 그러는데 리뷰를 받아야 하는 상태이지 않나요? |
@Lustellz 지난 회의 때, tag push 시점이 아닌 release published 시점에 워크플로우가 실행되도록 @hyoseong1994 님께서 프로세스 변경을 제안해주셨고, 그에 따라 @HowToBeAHappyBoy 님께서 본 PR에 반영해주시겠다고 한 걸로 기억하고 있습니다. |
1bfc48b to
5675164
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@hyoseong1994 효성님 안녕하세요! 코멘트 주신 부분 반영하였으며, PR 본문에 테스트와 관련된 내용을 추가했습니다 확인 한 번만 부탁드립니다 감사합니다 🙇 |
DaleSeo
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.
PR 설명에 테스트 내용 공유해주셔서 리뷰하는데 큰 도움이 되었습니다!
| @@ -0,0 +1,63 @@ | |||
| name: Npm Publish | |||
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.
다른 워크플로우들처럼 이름을 명사형으로 해주시고, 한 눈에 파악하기 좋도록 이모지 하나 달아주시면 좋을 것 같습니다.
| name: Npm Publish | |
| name: Publication 📦 |
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.
파일 이름도 publication.yml로 바꾸시죠.
| run: | | ||
| TAG_NAME="${{ github.event.release.tag_name }}" | ||
| TAG_VERSION="${TAG_NAME#v}" | ||
| PKG_VERSION=$(node -p "require('./package.json').version") |
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.
굳이 여기서 Node.js + CommonJS에 의존할 필요는 없을 것 같습니다.
| PKG_VERSION=$(node -p "require('./package.json').version") | |
| PKG_VERSION=$(jq -r .version package.json) |
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
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.
액션의 최신 버전을 사용해주세요.
| uses: actions/checkout@v4 | |
| uses: actions/checkout@v6 |
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
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.
최신 커밋만 필요한데 전체 Git 히스토리를 땡겨올 필요가 없지 않나요?
| fetch-depth: 0 |
| - name: Setup Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: "20" |
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.
| VERSION=${TAG_NAME#v} | ||
| echo "버전 ${VERSION} 배포를 시작합니다..." | ||
|
|
||
| npm publish --access public |
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.
우리 패키지는 scoped가 아니기 때문에 이 옵션은 필요 없습니다.
| npm publish --access public | |
| npm publish |
| if [ -z "${NODE_AUTH_TOKEN}" ]; then | ||
| echo "Error: NPM_TOKEN이 설정되지 않았습니다." | ||
| exit 1 | ||
| fi | ||
|
|
||
| TAG_NAME="${{ github.event.release.tag_name }}" | ||
| VERSION=${TAG_NAME#v} | ||
| echo "버전 ${VERSION} 배포를 시작합니다..." |
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.
이런 체크나 로깅은 그냥 생략하시죠? NODE_AUTH_TOKEN 없으면 어차피 npm publish 명령어가 실패할 것이고, 버전은 이미 전 단계에서 검증하고 로깅까지 했습니다.
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: "20" | ||
| registry-url: "https://registry.npmjs.org" |
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.
이 옵션을 위에 oven-sh/setup-bun@v2 액션에 설정해주시면, Bun과 Node.js 둘 다 설치 안 해도 될 것입니다. 추가로 필요한 변경들 아래 정리해드릴께요.
- 환경 변수 이름 변경:
NODE_AUTH_TOKEN->NPM_TOKEN - 발행 명령어 변경:
npm publish->bun publish
hyoseong1994
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.
리뷰하나 더 남겼는데 작업하였을 당시에는 없던 내역이라 우선 승인합니다. 작업 여부는 아카님이 판단하시고 알려주시면 후속 티켓 작성하는 등의 작업하도록하겠습니다
고생하셨습니다!
| - name: Build package | ||
| run: | | ||
| bun run prepare | ||
| bun run build |
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.
최근 작업한 내역에서 app 빌드와 lib 빌드과 나누어져 명령어가 bun run build:lib 로 변경되었습니다.
참고 PR입니다.
#685

변경 사항
목적
테스트
리뷰어에게
PR 작성자 체크 리스트