fix: 초성을 뜻하는 단어를 통일 #67
Conversation
🦋 Changeset detectedLatest commit: 62ec161 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@BO-LIKE-CHICKEN is attempting to deploy a commit to the Toss Team on Vercel. A member of the Team first needs to authorize it. |
| export function getFirstConsonants(word: string) { | ||
| return disassembleHangulToGroups(word).reduce((firstConsonants, [consonant]) => { | ||
| return `${firstConsonants}${consonant}`; | ||
| export function getChosung(word: string) { |
There was a problem hiding this comment.
이미 서비스에서 getFirstConsonants를 사용하고 있는 개발자분들은 괜찮으실까요?
패키지 업데이트 하면, 타입 에러가 날것이라 괜찮을 것 같기도한데요.
typescript가 아닌, javascript를 사용하시는 개발자분들은 런타임에러가 발생할것으로 보여서 이PR을 Breaking Change로 봐야할까요?
There was a problem hiding this comment.
javascript를 사용하는 사용자들은 미처 고려하지 못했네요 🥲
typescript 환경
먼저, 다음과 같은 이유로 'typescript 환경에서 사용하는 개발자들은 문제가 없을 것이다.' 라고 판단했어요.
- 가장 큰 이유는
utils.ts에 있는 함수들은 라이브러리 내부 구현을 위한 함수들이라고 생각했습니다. 문서에서도 그 정보를 제공하지 않기도 해서 사용자가 많지 않을 것이라고 생각했습니다. - @okinawaa 님이 말씀해주신 것처럼 타입 에러가 날 것이라 괜찮을 것 같다고 생각했습니다. 🙂
javascript 환경
이 부분은 고려하지 못했습니다. 😂 그렇다면 다음과 같은 방식으로 개선을 해나가면 좋을 것 같아요.
getFirstConsonants와getChosung을 모두 사용할 수 있도록 구현하되getFirstConsonants에는 TsDoc에deprecated정보를 제공해서 메이저 업데이트시에 제거한다.
추가로 개선이 필요한 부분
chosung,batchim등 한글을 영어로 그대로 표현한 경우를 모아둔 문서를 만든다면 앞으로 라이브러리 내에서 용어가 통일되며 한글이 익숙하지 않은 개발자들이 사용할때도 꽤나 도움이 되겠다는 생각이 듭니다.
결론
getFirstConsonants와getChosung을 모두 사용할 수 있도록 구현하되getFirstConsonants에는 TsDoc에deprecated정보를 제공해서 메이저 업데이트시에 제거한다.
해당 PR을 이렇게 개선한다면 Breaking Change로 판단하지 않고 하위호환을 지키며 앞으로 추가될 코드에서도 통일성있게 용어를 사용할 수 있을 것 같아요!
어떻게 생각하시나요?
There was a problem hiding this comment.
getFirstConsonants와 getChosung을 모두 사용할 수 있도록 구현하되 getFirstConsonants에는 TsDoc에 deprecated 정보를 제공해서 메이저 업데이트시에 제거한다.
너무 좋습니다! 명확한 해결책을 바로 제시해주셔서 감사해요!
There was a problem hiding this comment.
감사합니다! 🙇🏻♂️
조금 더 develop해서 리뷰 요청드리도록 할게요!
javascript 환경에서 라이브러리를 사용하는 유저에 대한 염려나, 하위호환성에 대한 문제를 함께 고민해 주셔서 감사합니다. 🤗
BO-LIKE-CHICKEN
force-pushed
the
refactor/standardize-naming-chosung
branch
from
497f041
to
546d51e
Compare
위와 같이 감사합니다! 🤗 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
- Coverage 99.00% 97.54% -1.46%
==========================================
Files 13 13
Lines 201 204 +3
Branches 45 45
==========================================
Hits 199 199
- Misses 2 5 +3 |
Overview
마이너한 수정사항 입니다. 🙂
#42 PR을 진행하면서 초성을 뜻하는 용어가 혼용되어 사용되고 있는 것을 발견했습니다. 이에 다음과 같은 수정사항을 반영했습니다.
initialConsonants로 사용되던 부분을chosung으로 변경firstConsonants로 사용되던 부분을chosung으로 변경PR Checklist