fix: 일관된 한글 표기법 적용 #138
fix: 일관된 한글 표기법 적용 #138
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/index.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| export * from './assemble'; | |||
| export * from './chosungIncludes'; | |||
| export * from './choseongIncludes'; | |||
There was a problem hiding this comment.
아직 BC를 발생시키기 전이기때문에,
chosungIncludes는 라이브러리를 사용하는 사람들이 안전하게 사용하도록 chosungIncludes, choseongIncludes 을 동시에 export 해주고,
2버전으로 넘어갈때, chosungIncludes를 제거하는것은 어떤가요?
그전에 chosungIncludes 에 jsdoc으로 deprecated 주석을 남겨두는것도 좋을 것 같아요
There was a problem hiding this comment.
chosung~ 버전의 함수와 choseong~ 버전의 함수가 일단 공존하는게 좋다는 말씀일까요??
만약 말씀을 잘 이해한 것이라면 2개 버전의 함수가 모두 존재하고, export도 하는 것으로 작업하겠습니다 !!
There was a problem hiding this comment.
네네 맞습니다! 오픈소스이다보니,
major update를 하지않았는데도 불구하고, 유저가 서비스의 코드를 수정해야하는상황을 막고 싶었어요!
There was a problem hiding this comment.
chosungIncludes에는 jsdoc으로 deprecated와 choseongIncludes로 사용할 수 있게 처리해주시면 감사하겠습니다
| @@ -1,7 +1,7 @@ | |||
| import React, { useState } from 'react'; | |||
| import { chosungIncludes } from 'es-hangul'; | |||
|
|
|||
| export function ChosungIncludesDemo() { | |||
| export function chosungIncludesDemo() { | |||
| const [searchWord, setSearchWord] = useState<string>('홍길동'); | |||
There was a problem hiding this comment.
이거 소문자로 바꾸신 이유가 있으신가요?
이 부분도 sung이 아닌, seong이 되면 좋겠어요!
There was a problem hiding this comment.
네 알겠습니다 ! 👍
해당 PR에서 hangul을 제외하는 것도 진행해도 괜찮은지 여쭙고 싶습니다 !
만약 PR을 분리해야 한다면, 제가 맡고 싶습니다 :)
There was a problem hiding this comment.
PR을 분리하면 좋을 것 같아요!
맡아주신다니 영광입니다, BC가 없도록 두가지 메서드씩 모두 살려두는 방향으로 작업해주시면 좋을 것 같아요!
There was a problem hiding this comment.
sung => seong 이 작업 및 컨플리긑 해결 부탁드립니다 🙏
|
안녕하세요 ! @okinawaa 
Jongsung, Chosung 등을 사용할 수 없다고 오류가 발생해요 ! |
There was a problem hiding this comment.
아직 *sung 으로 남는 표기가 많은 것 같아요..! 확인 한 번 부탁드립니다!
또한 eslint 에러로 표기되는 이슈는 #172 이 merge 되면 warning 으로 반영된다는 점 참고 부탁드립니다!
There was a problem hiding this comment.
deprecated 되었다는 표기를 남기고 새로운 문서를 만드는 것이 좋지 않을까요? 🤔
패키지 상에서는 deprecated 인데, 문서는 없어지는 것이 우려됩니다.
There was a problem hiding this comment.
리뷰해주셔서 감사합니다 ! @minsoo-web
chosungIncludes는 라이브러리를 사용하는 사람들이 안전하게 사용하도록 chosungIncludes, choseongIncludes 을 동시에 export 해주고,
chosung~과 choseong~ 모두 작성하는 것으로 위 문장을 이해했는데 잘못 이해한 부분이 있다면 말씀해주시면 감사하겠습니다 !
새로운 문서를 만드는 것이 좋지 않을까요? 🤔
문서 또한 chosung~과 choseong~ 모두 작성하는 것으로 진행할까요??
There was a problem hiding this comment.
chosung
과 choseong모두 작성하는 것으로 위 문장을 이해했는데 잘못 이해한 부분이 있다면 말씀해주시면 감사하겠습니다 !
네네! 같은 방향으로 이해한 것 같습니다.
다만, chosung 이 잘못된 표기니까, deprecated 처리 하되 문서는 url이 chosung 이 포함된 url 이 있다면, choseong 으로 이동할 수 있도록 가이드 하면 어떨까요?
예시)
기존:
https://es-hangul.slash.page/docs/api/chosungIncludes
변경 후:
https://es-hangul.slash.page/docs/api/chosungIncludes (deprecated) > choseongIncludes 를 사용해주세요!
https://es-hangul.slash.page/docs/api/choseongInclues
이런 느낌이죠!!
아니면, deprecated features 라는 페이지를 만들어서 기재해두어도 괜찮을 것 같아요!
ref: https://v2.chakra-ui.com/getting-started/migration#5-deprecated-features
There was a problem hiding this comment.
넵 알겠습니다 !
chosung은 deprecated 처리chousng의 문서는 삭제하지 않고choseong과 함께 존재
정리하자면 위와 같이 진행하면 되는 것일까요??
There was a problem hiding this comment.
제가 바라는 방향성입니다!!
문의 주신 내용에 대해서는 @okinawaa 님이 답변 주시면 너무 좋을 것 같아요!!
제가 확답 드리지 못하는 이유는, 저는 메인테이너도 코드오너도 아니기 때문입니다 !!
There was a problem hiding this comment.
넵 저도 @minsoo-web 님 의견 너무 좋습니다~ 문서도 살려두면 좋겠네요!
#172 이 머지되어서 해결되었을 것 같아요! 감사합니다. |
|
안녕하세요 @okinawaa ! 작업 완료 했습니다 ! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
===========================================
- Coverage 100.00% 94.20% -5.80%
===========================================
Files 16 17 +1
Lines 260 276 +16
Branches 58 59 +1
===========================================
Hits 260 260
- Misses 0 15 +15
- Partials 0 1 +1 |
Overview
chosung으로 안내하도록 수정했습니다.sung~,seong~을 혼용했고,sung~함수에@deprecated를 적용했습니다.seong을 사용했습니다.PR Checklist
close #131