작년(2023) 우아한테크코스를 하면서 처음으로 DroidKnights 컨퍼런스를 알게 되었고, 크루들과 함께 다녀온 경험이 있다.
DroidKnights는 국내 최대 규모의 안드로이드 컨퍼런스이다.
컨퍼런스를 편하게 즐길 수 있도록, 기관 내에서 자체적으로 컨퍼런스의 세션 목록을 조회하고 북마크할 수 있는 앱을 운영한다.
운 좋게도 올해 DroidKnights 앱 개발에 공식적으로 참여할 수 있는 기회가 생겨 바로 도전해보았다.
컨퍼런스 운영진분들께서 기존 코드에 대한 리팩터링이나 기능 추가에 대한 티켓을 만들어 놓으신다.
그러면 원하는 이슈에 누구나 Comment를 작성하여 참여 의사를 밝히면 된다.
최근에 Compose를 사용하여 펫튜디오라는 서비스를 개발하면서, 컴포넌트 분리를 해본 경험이 있기 때문에 UI 파일을 단위별로 분리 하는 이슈에 가장 관심이 갔다.
바로 코멘트를 작성하여 참여의사를 밝혔고, 얼마 지나지 않아 2024 레이블과 Assign을 해주셨다.
공통 컴포넌트를 찾아 분리하고, 하나의 파일에 너무 많은 컴포넌트가 몰려 있는 코드를 개선하는 리팩터링이었기에 기존에 작성되어 있던 코드에 대한 충분한 이해가 필요했다.
컨트리뷰트 과정을 간략하게 요약하면 다음과 같다.
- (멀티 모듈이었기에) feature 모듈에 작성된 UI 코드를 분석한다.
- 분리할 컴포넌트를 찾아 함수 또는 별도의 파일로 분리한다.
- 코드가 기존과 동일한 동작을 하는지 확인한다.
- 코드 리뷰 및 코멘트 반영
1. feature 모듈에 작성된 UI 코드 분석
타 프로젝트에 기여하기 위한 목적으로 코드를 수정하는 일은 처음이었기 때문에,
버그가 발생하지 않도록 작업 내내 긴장하며 최대한 실수하지 않으려고 했다.
화면은 크게 6개로 분리되어 있었으며 앱의 규모가 크지 않았다.
컴포넌트 분리 기준이 아직 명확하지 않았기 때문에, 모듈을 하나씩 열어보며 코드를 분석하는 과정에서 기준을 세우고자 했다.
(레퍼지토리가 public이기는 하지만, 내부 소스 코드의 공개 가능 여부를 알지 못하기 때문에 링크만 첨부합니다.)
개인적인 생각으로는 이미 home 모듈의 분리가 잘 이루어져 있어 보였기 때문에 해당 모듈을 기준으로 삼았다.
이 외에도, 다른 일부 모듈에는 Text, Image 하나로 이루어진 컴포넌트들도 적절한 이름으로 각각 분리되어 있는 사례가 있었다.
따라서, 처음으로 고려한 컴포넌트 분리 기준은 다음과 같다.
- Text, Image 등 하나로 이루어진 뷰들도 모두 적절한 이름을 갖는 함수로 분리
- XxxScreen(전체 화면 구성) 파일에 코드가 많다보니, 하위 컴포넌트와의 연관성을 기준으로 별도의 파일로 분리
2. 분리할 컴포넌트를 찾아 함수 또는 별도의 파일로 분리
setting 화면의 경우, 오픈소스 라이센스 카드 는 별도의 파일로 분리되어 있었지만,
그 아래 테마 설정 카드 는 SettingScreen.kt 파일 내부에 작성되어 있었다.
이러한 코드를 ThemeCard.kt 파일로 분리하여 별도로 관리할 수 있도록 변경하였다.
그리고 그 안에서도 Text, Image, RadioButton 등을 각 역할과 이름에 맞도록 SettingTitle(), ThemeImage(), ThemeName() 등으로 분리하였다.
이 외에 공통적으로 사용할 수 있는 컴포넌트는 designsystem 모듈에 추가하였다.
예를 들어, 테두리가 둥근 이미지를 표현할 수 있는 RoundedImage, Depth를 1만큼만 사용하는 커스텀 IconToggleButton 등이 있다.
다만, 분리하고 보니 해당 컴포넌트들은 어차피 한 곳에서만 사용되고 있었기에 designsystem으로 분리할만큼의 가치가 있는가? 에 대한 의문이 들었다.
그래도 언젠가 공통적으로 사용될 수 있을 것이라고 생각이 들어 커밋을 해두었다.
그리고 분리를 하면서 만들어진 컴포넌트 + 이미 분리되어 있지만 Preview가 없는 컴포넌트에 대해서 Preview를 작성해두었다.
이 과정에서 PreviewParameterProvider 를 처음 알게 되어 용이하게 사용하였다.
3. 코드가 기존과 동일한 동작을 하는지 확인
기능을 추가하는 것이 아닌, 구조를 변경하는 작업이었기 때문에 조금 더 신중했다.
하나의 컴포넌트를 기준으로 코드를 수정하고, 수정 이전의 기능과 동일하게 동작하는지 확인하며 점진적으로 리팩터링하였다.
일부 화면에 대해서는 UI Test 코드가 작성되어 있었지만, 그렇지 않은 화면들도 있었기에 모든 화면이 올바르게 동작하는지 알아볼 수는 없었다.
또한, Test 코드는 도구이지 이것을 100% 신뢰할 수는 없다는 입장이었기에 QA하는 마음으로 직접 앱을 실행하며 동작을 확인하는 과정을 거쳤다.
이렇게 꼼꼼히 확인을 하다보니 컴포넌트 분리 후에 일부 화면이 작아지거나 크게 보이는 등의 문제를 캐치할 수 있었다.
보통은 분리 과정에서 하위 컴포넌트가 외부에서 주입받은 부모의 modifier 객체를 그대로 사용하여 문제가 발생한 경우였다. 이전 프로젝트에서도 비슷한 경우가 있어서 금방 알아챌 수 있었다.
4. 코드 리뷰 및 코멘트 반영
작업한 내용을 요약하고 TO-DO 형식으로 작성하여 리뷰어분께서 최대한 보시기 편하게 작성하였다.
첫 컨트리뷰트를 위한 PR이다 보니, 평소에는 아무렇지 않게 누르던 버튼 하나하나까지도 긴장의 끈을 놓치지 않고 10초 이상 생각하며 눌렀다.
나름 고민을 많이 하고 최선을 다했다고 생각했기 때문에 긴장을 머금은 뿌듯함과 함께 PR 제출 버튼을 눌렀다.
하지만 뿌듯한 마음은 잠시, 얼마 지나지 않아 분리한 구조가 과하다는 리뷰를 받았다.
이 외에도 추가적인 리뷰가 있었으며, 요약하자면 다음과 같다.
- Text, Image 등은 이미 스타일이 적용되어 있어서 굳이 분리할 필요가 없다.
- 재사용 가능성을 고려하지 않은 분리처럼 보인다.
- 하나의 PR에 너무 많은 내용이 담겨 있다.
솔직한 마음으로는 처음 리뷰를 받았을 때 스스로에 대한 아쉬움이 컸고,
기존 코드의 분리된 구조를 레퍼런스 삼아 작업했기 때문에 어떻게 수정해야 할지 조금은 막막했다.
우선은 PR 분리에 대해 먼저 고민해보았고, 제안해주신 순서대로 작업을 재진행하였다.
공통 컴포넌트 분리 -> feature 모듈 컴포넌트 분리 -> 분리한 컴포넌트 Preview 작성
공통 컴포넌트 분리
결론부터 말하자면 공통 컴포넌트 분리 PR은 반영되지 않았다.
IconToggleButton(), RoundedImage() 를 만들어 분리하였지만, 한 곳에서만 사용되고 있으며 공통적으로 사용하기에는 설정값에 애매한 부분이 많았기 때문이다.
feature 모듈 컴포넌트 분리
이전 PR에서 문제라고 생각했던 Text, Image 각각의 컴포넌트까지 함수로 분리했던 부분을 해결하고 PR을 올렸다.
또한, 어느정도는 기존 코드와의 통일성을 지키고자 LazyColumn 내부의 items() 함수를 별도로 분리하였다.
커밋도 최대한 작업 단위별로 하나씩 쪼개어 가며 문제가 생겼을 때 돌아갈 수 있도록 섬세하게 작업했다.
하지만 리뷰어님으로부터 아직 불필요한 분리가 많다는 리뷰를 받았다.
과연 어디까지 분리해야 하는가? 에 대해 충분히 고민했다고 생각했지만 아직 부족함이 많이 보인 듯 했다.
기존 코드의 분리된 구조를 레퍼런스 삼아 작업했기에, 여기서 어떤 방향으로 나아갈지 떠올리는 것이 매우 어려웠다.
이 시점에 나의 의견을 남기지 않으면, 기여를 하며 얻는 배움도 없을 뿐더러, 나중에 의견을 남기지 않은 것에 대해 후회할 것 같았다.
따라서 작업을 하며 느낀 함수 분리의 목적 에 대해 작성하였고, 마지막엔 리뷰어님의 의견을 한 번 더 여쭤보았다.
말씀해주신 내용을 다음과 같이 이해하였다.
- Text는 Style을 적용하여 공통화했기 때문에 이미 Text() 만으로 분리된 컴포넌트 역할을 한다.
- Text/Image 컴포넌트 내부 구성은 효율적이기에 분리하지 않아도 된다.
- 의미있는 컴포넌트 분리를 통해 화면을 조립하는 형태로 사용해야 한다. (LazyColumn의 Item 컴포넌트 처럼)
- Text/Image 가 분리된 형태를 모두 Preview로 작성할 필요는 없다. 잘 만들어진 컴포넌트 하나에 대한 Preview면 충분하다.
- 과도한 분리는 이후에 테스트, 디자인 수정을 어렵게 만든다.
리뷰를 읽다보니 내가 너무 기존 코드와의 통일성만을 고려했다고 느껴졌다.
물론 통일성을 지키는 것이 여전히 나쁘다고 생각하지는 않지만 컨트리뷰트 + 리팩터링인만큼 나의 주관을 코드에 한 숟가락 정도는 더 담았어도 괜찮았을 것이라는 생각이 들었다.
이후에는 코드 리뷰에서 이해한 내용을 바탕으로 화면을 구성하기에 조금 더 의미있는 형태로 리팩터링하고자 불필요한 분리를 제거하였다.
- 기존에 작성되어 있던 코드 중에서도 단순 Text, Image인데도 큰 의미 없이 분리되어 있는 부분을 일부 수정했다.
- 동일한 컴포넌트 코드를 감싸고 있는 함수를 제거하였다.
@Composable private fun XxxItem(...) { XxxCard(...) } - Skeleton을 Spacer()로 구현하고 있었기에 이를 함수로 분리하여 재사용 가능하게끔 수정하였다.
그리고 마침내 처음으로 PR이 merge되었다.
정식으로 DroidKnights의 Contributor가 된 것이다.
모든 PR에 남겨주시는 멘트라는걸 알면서도 고생하셨습니다. 라는 한 마디가 나에게는 큰 위로와 힘이 되었다.
분리한 컴포넌트 Preview 작성
이렇게 끝인 줄 알았지만 분리한 컴포넌트에 대해 Preview를 작성해야 하는 작업이 남아 있었다.
다행히 Preview 작성은 작업의 목표가 명확했기에 비교적 수월하게 PR을 올릴 수 있었다.
@Composable
@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES)
@Preview(uiMode = Configuration.UI_MODE_NIGHT_NO)
private fun XxxPreview() {
KnightsTheme {
Xxx()
}
}각 컴포넌트의 LightMode와 DarkMode에 대한 Preview를 작성하였다.
Preview 애노테이션에는 다양한 파라미터를 지정할 수 있기 때문에 함수를 여러개 만들지 않고도 작업할 수 있었다.
감사하게도 PreviewParameterProvider 에 대한 공식문서 링크까지 첨부하여 리뷰해주시면서 바로 merge가 되었다.
화면에 여러개의 데이터를 보여줄 때에만 해당 클래스를 사용한다고 생각했는데,
생각해보면 샘플 데이터 관리를 PreviewParameterProvider로 하면 이후에 보여줄 데이터가 추가될 때 큰 수정이 없어도 되고, 샘플 데이터를 분리하여 관리할 수 있기 때문에 샘플 데이터와 UI 코드에 대한 가독성도 각각 높일 수 있을 것이다.
느낀점
PR 관점
처음에 욕심을 부려 하나의 PR에 너무 많은 내용을 담다보니 리뷰어분께서 리뷰하시기 힘드셨겠다는 생각이 든다.
PR을 목적에 맞게 분리해야 리뷰어와 리뷰이 모두 작업 내용을 명확하게 이해할 수 있을 것이라 느꼈다.
한 PR에 목적과 약간이라도 다른 작업이 추가되면, 이후에 문서로서의 가치도 떨어질 것이라는 우려가 들었다.
코드 관점
컴포넌트 분리에 대해 깊이 고민할 수 있었고, 다른 사람이 작성한 코드를 분석하면서 다양한 코드 작성법에 대해서도 배울 수 있었다.
특히 기억에 남는 코드 구조는 대략 다음과 같다.
LazyColumn {
itemsIndexed { index, item ->
if (index == 0) {
Header()
}
ItemComponent()
}
}나는 지금까지 Header()와 같은 구조를 item { Header() }로 작성하는 경우가 많았는데, 이번 기회를 통해 같은 기능이라도 여러 종류의 코드로 표현할 수 있음을 다시 한 번 깨달았다.
전체적으로는 처음으로 외부 코드에 기여했다는 점에서 뿌듯하면서도, 리뷰어님께 부족한 모습을 많이 보여드린 것 같아 아쉬움도 많이 남았다.
그러나 나의 의견을 코멘트에 남기고 리뷰어와 의견을 주고받는 과정을 통해 점점 개발자로서의 정체성이 형성되고 있다는 기분이 들었다.
부족한 점을 인정하고 앞으로 지금보다 더 잘해야겠다는 동기부여가 되었다.
Github와 DroidKnights 앱 Contributor 화면에 추가된 모습으로 회고 마무리.. 😁
화이팅입니다!