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

feat: 린트 액션을 추가합니다 #27

Merged
merged 3 commits into from
Jun 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: lint

on:
pull_request:

jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2

- name: Set up JDK 17
uses: actions/setup-java@v1
with:
java-version: 17

- name: Grant execute permission for gradlew
run: chmod +x gradlew

- name: Test with Ktlint
run: ./gradlew --info ktlintCheck
Copy link

Choose a reason for hiding this comment

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

코드 패치를 검토해보면 다음과 같은 피드백을 드릴 수 있습니다:

  1. 파일의 마지막에는 항상 새줄(newline character)을 추가해주세요. 이는 Unix/Linux 스타일의 텍스트 파일에서 관례처럼 따르는 규칙입니다.
  2. actions/setup-java@v1 대신 actions/setup-java@v2 를 사용하는 것을 추천합니다. v2 버전은 병렬적인 작업이 가능하며, 보안 강화, 버그 픽스 등의 이점이 있습니다.
  3. Gradle Wrapper Validation을 고려해 보세요. 이는 서드파티가 조작하지 못하도록 Gradle Wrapper를 보호해줍니다.

수정 제안:

@@ -0,0 +1,23 @@
+name: lint
+
+on:
+  pull_request:
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      
+      - name: Validate Gradle Wrapper
+        uses: gradle/wrapper-validation-action@v1
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v2
+        with:
+          java-version: 11
+
+      - name: Grant execute permission for gradlew
+        run: chmod +x gradlew
+
+      - name: Test with Spotless
+        run: ./gradlew --info ktlintCheck

Copy link

Choose a reason for hiding this comment

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

이 코드 패치에 대해 간략히 살펴보면 다음과 같습니다:

  1. 파일의 마지막에 개행 문자가 없습니다. 이는 일반적으로 스타일 가이드에서 권장하는 것 중 하나인데, 이로 인해 diff 출력을 읽거나 리다이렉트할 때 실수를 줄일 수 있습니다.

  2. JDK 17을 사용하도록 설정되어 있습니다. 이것이 프로젝트의 필요한 Java 버전인지 확인하십시오.

  3. 작업 이름이 'lint'라고 되어 있는데, 실제로는 Ktlint를 통해 코드 검사를 진행하고 있습니다. 더 명확하게 작업 이름을 설정하여 해당 작업이 실제로 어떤 작업을 수행하는지 명확하게 드러내도록 합니다.

  4. actions/checkout@v2actions/setup-java@v1와 같은 액션을 사용할 때 가능하면 최신버전을 사용하는 것이 좋습니다. 그러나 이는 현재 당신의 프로젝트에서 호환성 문제를 발생시키지 않는 경우에만 적용됩니다.

  5. 본 코드에서 실패 시 CI 파이프라인이 바로 중단된다는 점을 고려하였는지 생각해보세요. Gradle task가 실패하면 이후 스텝들은 수행되지 않습니다. 이것이 기대하는 동작인지 확인하십시오.

  6. 마지막으로, --info 옵션을 사용해서 세부 내용까지 로그에 남기고 있습니다. 이 부분은 디버깅에 도움이 될 수 있지만 불필요하게 많은 정보를 로그에 남길 수도 있으니 운영 환경에서 필요한 만큼의 로그만 출력되도록 조절하는 것이 좋습니다.

2 changes: 1 addition & 1 deletion api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ dependencies {
/** spring starter */
implementation("org.springframework.boot:spring-boot-starter-webflux")
implementation("org.springframework.boot:spring-boot-starter-actuator")
}
}
1 change: 0 additions & 1 deletion data/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,3 @@ dependencies {
implementation("org.flywaydb:flyway-core:${DependencyVersion.FLYWAY}")
implementation("org.flywaydb:flyway-mysql")
}

Copy link

Choose a reason for hiding this comment

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

이 패치의 변경 사항은 두 가지 입니다:

  1. org.flywaydb:flyway-mysql 추가: 이 의존성은 FlywayDB에서 MySQL을 지원하기 위한 것으로 보입니다. 해당 데이터베이스를 사용하는 경우에만 필요합니다.
  2. 불필요한 공백 라인 제거: 깔끔함과 코드 일관성 측면에서는 향상되었지만, 잠재적인 문제나 결함에 대해서는 영향을 주지 않습니다.

버그 리스크를 관리하려면, 이 변경 사항을 테스트 환경에서 적용하고, 테스트 케이스 실행을 통해 요구 사항을 충족하는지 검증해야 합니다.

더 개선시키려면, 버전 관리를 위해 ${DependencyVersion.FLYWAY} 치환자를 사용하고 있는데, flyway-mysql에도 동일한 방식으로 버전 관리를 할 수 있는지 확인해야 합니다. 각 패키지가 자체 버전 관리를 하고 있다면, 이 점을 고려하아야 합니다. 그렇지 않으면, 일부 패키지 업데이트시 연관 패키지와 호환성 문제가 발생할 수 있습니다.

Loading