CI 스크립트 개선 (issue #108) #111
Conversation
|
지나가다 들려요. |
오 좋네요 ㅎㅎ 측정해보고 결과 붙여서 pr 업데이트 해놓겠습니다 |
lilychoibb
force-pushed
the
refactor/#108
branch
from
4b3b6f1
to
36f63a8
Compare
|
|
.github/workflows/backend_ci.yml
Outdated
| - name: Gradle Caching | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: | | ||
| ~/.gradle/caches | ||
| ~/.gradle/wrapper | ||
| key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gradle- | ||
|
|
||
| - name: Setup Gradle | ||
| run: chmod +x ./backend/gradlew |
There was a problem hiding this comment.
There was a problem hiding this comment.
수동으로 작업해주지 않아도 되었군요 👍 반영해놓겠습니다~
.github/workflows/backend_ci.yml
Outdated
| script: | | ||
| const fs = require('fs'); | ||
| const workers = JSON.parse(fs.readFileSync('.github/workflows/teamMember.json')); | ||
| const mention = context.payload.pull_request.assignees.map((user) => { | ||
| const login = user.login; | ||
| const mappedValue = workers[login]; | ||
| return mappedValue ? `<@${mappedValue}>` : `No mapping found for ${login}`; | ||
| }) | ||
| return mention.join(', '); |
There was a problem hiding this comment.
[Approve]
현재 팀 멤버는 변경될 가능성이 굉장히 적은데 차라리 상단에 env나 상수로 정의하는 건 어떨까요?
There was a problem hiding this comment.
json 을 따로 분리해두어 사용하는 방식은 파일에 접근하고 추가적인 I/O 를 위한 작업을 해야하긴 하지만 데이터와 로직을 분리해둔다는 장점이 있는 것 같아요!
리브가 제안주신 스크립트 하나에서 처리하는 방식은 한 곳에서 관리할 수 있다는 장점이 있지만 env 로 정의하더라도 readFileSync 만 수행하지 않을 뿐 json parse 를 해야하고, 상수는 스크립트 내에 정의해두면 스크립트가 너무 길어질 것 같아요.
말씀주신 것과 다른 내용이긴 하지만 위의 스크립트가 지금도 충분히 길어서 계속 개선점을 찾고 있는데,, 아직은 쉽지 않네요 🥹
There was a problem hiding this comment.
assignee -> sender 로 변경 후 수정
.github/workflows/backend_ci.yml
Outdated
| server-id: github # Value of the distributionManagement/repository/id field of the pom.xml | ||
| settings-path: ${{ github.workspace }} # location for the settings.xml file |
There was a problem hiding this comment.
[Comment]
이 부분은 Maven 빌드할 때 사용되는 부분인 것으로 알고 있습니다!
구현 요약
연관 이슈
close #108
참고
코드 리뷰에
RCA 룰을 적용할 시 참고해주세요.