Conversation
Summary of ChangesHello @koosco, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 프로젝트의 아키텍처 무결성을 강화하고 개발 워크플로우를 자동화하는 데 중점을 둡니다. ArchUnit을 도입하여 계층 간 및 도메인 간의 의존성 규칙을 코드 레벨에서 강제함으로써, 장기적인 유지보수성과 확장성을 향상시키고 아키텍처의 일관성을 보장합니다. 또한, Claude AI 훅을 통해 코드 포맷팅 및 테스트 실행을 자동화하여 개발 생산성을 높이고 잠재적인 오류를 조기에 발견할 수 있도록 합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Code Format Check ✅
|
Test ✅
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 ArchUnit을 사용하여 아키텍처 의존성 규칙을 테스트로 명시화하고, Claude Code hook을 추가하여 개발 프로세스를 자동화하는 좋은 변경입니다. 하지만 .claude/settings.json 파일에서 환경 변수가 셸 명령에서 안전하지 않게 사용되어 공격자가 임의 코드를 실행할 수 있는 높은 심각도의 명령 주입 취약점이 발견되었습니다. 이 취약점은 변수를 적절히 살균하거나 인용하여 해결하는 것이 권장됩니다. 또한, 새로 추가된 ArchitectureRulesTest.kt 파일의 코드 중복을 줄이고 유지보수성을 높이기 위한 몇 가지 리팩토링 제안이 있습니다. 전반적으로 훌륭한 기여입니다!
| noClasses() | ||
| .that().resideInAnyPackage("..domain..") | ||
| .should().dependOnClassesThat().resideInAnyPackage( | ||
| "com.neki.auth.api..", | ||
| "com.neki.user.api..", | ||
| "com.neki.photo.api..", | ||
| "com.neki.media.api..", | ||
| "com.neki.pose.api..", | ||
| "com.neki.map.api..", | ||
| "com.neki.term.api..", | ||
| "com.neki.version.api..", | ||
| ).because("Domain layer must not depend on API layer (common.api is allowed)") | ||
| .check(importedClasses) |
There was a problem hiding this comment.
하드코딩된 패키지 목록은 유지보수를 어렵게 만들 수 있습니다. DomainIsolation 클래스에 정의된 allDomainPackages 리스트를 클래스 레벨로 올리고, 이를 사용하여 동적으로 API 패키지 목록을 생성하는 것을 고려해보세요. 이렇게 하면 도메인이 추가/삭제될 때 한 곳만 수정하면 되어 유지보수성이 향상됩니다.
예시:
// ArchitectureRulesTest 클래스 레벨에 정의
private val allDomainPackages = listOf(
"com.neki.auth",
// ... 다른 도메인
)
private val domainApiPackages = allDomainPackages.map { "$it.api.." }.toTypedArray()
// 테스트 내부
noClasses()
.that().resideInAnyPackage("..domain..")
.should().dependOnClassesThat().resideInAnyPackage(*domainApiPackages)
// ...이 패턴을 Application 계층은 도메인별 API 계층을 의존할 수 없다 와 Infra 계층은 도메인별 API 계층을 의존할 수 없다 테스트에도 동일하게 적용할 수 있습니다.
| @Test | ||
| fun `photo 도메인은 다른 도메인에 의존할 수 없다`() { | ||
| noClasses() | ||
| .that().resideInAnyPackage( | ||
| "com.neki.photo.api..", | ||
| "com.neki.photo.application..", | ||
| "com.neki.photo.domain..", | ||
| ).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages("photo")) | ||
| .because("photo domain (api/application/domain) must not depend on other domains") | ||
| .check(importedClasses) | ||
| } | ||
|
|
||
| @Test | ||
| fun `media 도메인은 다른 도메인에 의존할 수 없다`() { | ||
| noClasses() | ||
| .that().resideInAnyPackage( | ||
| "com.neki.media.api..", | ||
| "com.neki.media.application..", | ||
| "com.neki.media.domain..", | ||
| ).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages("media")) | ||
| .because("media domain (api/application/domain) must not depend on other domains") | ||
| .check(importedClasses) | ||
| } | ||
|
|
||
| @Test | ||
| fun `pose 도메인은 다른 도메인에 의존할 수 없다`() { | ||
| noClasses() | ||
| .that().resideInAnyPackage( | ||
| "com.neki.pose.api..", | ||
| "com.neki.pose.application..", | ||
| "com.neki.pose.domain..", | ||
| ).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages("pose")) | ||
| .because("pose domain (api/application/domain) must not depend on other domains") | ||
| .check(importedClasses) | ||
| } | ||
|
|
||
| @Test | ||
| fun `term 도메인은 다른 도메인에 의존할 수 없다`() { | ||
| noClasses() | ||
| .that().resideInAnyPackage( | ||
| "com.neki.term.api..", | ||
| "com.neki.term.application..", | ||
| "com.neki.term.domain..", | ||
| ).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages("term")) | ||
| .because("term domain (api/application/domain) must not depend on other domains") | ||
| .check(importedClasses) | ||
| } | ||
|
|
||
| @Test | ||
| fun `version 도메인은 다른 도메인에 의존할 수 없다`() { | ||
| noClasses() | ||
| .that().resideInAnyPackage( | ||
| "com.neki.version.api..", | ||
| "com.neki.version.application..", | ||
| "com.neki.version.domain..", | ||
| ).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages("version")) | ||
| .because("version domain (api/application/domain) must not depend on other domains") | ||
| .check(importedClasses) | ||
| } |
There was a problem hiding this comment.
도메인 격리 규칙을 검증하는 테스트들(photo, media, pose, term, version)이 거의 동일한 구조를 가지고 있습니다. 코드 중복을 줄이고 새로운 도메인을 추가할 때 더 쉽게 관리할 수 있도록 private helper 메서드를 만들어 리팩토링하는 것을 고려해 보세요. 이렇게 하면 규칙 로직이 한 곳에 모이게 되어 유지보수가 용이해집니다.
@Test
fun `photo 도메인은 다른 도메인에 의존할 수 없다`() {
checkDomainIsolation("photo")
}
@Test
fun `media 도메인은 다른 도메인에 의존할 수 없다`() {
checkDomainIsolation("media")
}
@Test
fun `pose 도메인은 다른 도메인에 의존할 수 없다`() {
checkDomainIsolation("pose")
}
@Test
fun `term 도메인은 다른 도메인에 의존할 수 없다`() {
checkDomainIsolation("term")
}
@Test
fun `version 도메인은 다른 도메인에 의존할 수 없다`() {
checkDomainIsolation("version")
}
private fun checkDomainIsolation(domain: String) {
noClasses()
.that().resideInAnyPackage(
"com.neki.$domain.api..",
"com.neki.$domain.application..",
"com.neki.$domain.domain..",
).should().dependOnClassesThat().resideInAnyPackage(*otherDomainPackages(domain))
.because("$domain domain (api/application/domain) must not depend on other domains")
.check(importedClasses)
}
Code Format Check ✅
|
Test ✅
|
Code Format Check ✅
|
Test ✅
|
Darren4641
left a comment
Darren4641
left a comment
There was a problem hiding this comment.
사용하면 좋은 것 같습니다! 휴먼에러를 줄일 수 있을 것 같아요 LGTM !
2 participants
summary
details
계층간 의존성을 명시적으로 추가
claude code spotless, test hooks 추가