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: 한글의 두음을 반환해주는 acronymizeHangul 함수를 제거합니다. #180

Merged
merged 5 commits into from
Jul 14, 2024

Conversation

okinawaa
Copy link
Member

Overview

#176 에서 논의한 결과
acronymizeHangul은 한글의 원리를 해결하는것이 아닌, 자바스크립트 일반 메서드로도 해결이 가능하므로 es-hangul에서 제공해주지 않아도 된다고 생각합니다.

s.split(' ').map(s => s[0]).join('')를 하면 사용자 쪽에서도 사실 외부 의존성 없이 해결할 수 있어요. 한글이 아닌 문자는 취급하지 않는다고 써있는 es-hangul 메서드와 다르게 다른 종류의 문자열을 주더라도 잘 동작(내지는 어떻게 동작할지 예측이 가능)하기도 하구요.

PR Checklist

  • I read and included theses actions below
  1. I have read the Contributing Guide
  2. I have written documents and tests, if needed.

@okinawaa okinawaa requested a review from raon0211 as a code owner July 12, 2024 14:51
Copy link

vercel bot commented Jul 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
es-hangul ❌ Failed (Inspect) Jul 14, 2024 7:52am

Copy link

changeset-bot bot commented Jul 12, 2024

🦋 Changeset detected

Latest commit: 4a4c0d8

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

@okinawaa
Copy link
Member Author

@KNU-K 님 안녕하세요. #133 에서 정말 많은 작업해주셨는데요,
이 PR에 대해서 어떻게 생각하시는지 궁금해서 멘션드립니다!

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.05%. Comparing base (84eead8) to head (7c8bcab).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##               v2     #180      +/-   ##
==========================================
- Coverage   95.08%   95.05%   -0.04%     
==========================================
  Files          15       14       -1     
  Lines         285      283       -2     
  Branches       67       67              
==========================================
- Hits          271      269       -2     
  Misses         13       13              
  Partials        1        1              

@okinawaa okinawaa requested a review from manudeli July 12, 2024 15:02
@KNU-K
Copy link
Contributor

KNU-K commented Jul 13, 2024

제가 느끼기에는 acronymizeHangul이라는 함수는 편의성에 따른 유틸이라고 생각이 됩니다.
C++ 로 예를 들어보면 해당 함수 동작에 대한 것을 꽤나 간단하게 만들 수 있지만, 이를 제공하는 것을 볼 수 있습니다.
acronymizeHangul도 그와 비슷한 느낌이라고 생각됩니다! 이 부분에서 어떻게 생각하시나요?

@KNU-K
Copy link
Contributor

KNU-K commented Jul 13, 2024

약간 방향성이 해당 라이브러리에서만 볼 수 있는 한글처리 기술에 방향성을 둔 것이라면 지워져도 상관없을 것 같긴 합니다.

@okinawaa
Copy link
Member Author

@KNU-K 각 문자의 앞 글자만 따는 행위는, 한글의 복잡한 초,중,종성 자음 모음 규칙을 알지 못하는 사용자라도
쉽게 구현할 수 있어요. 그래서 es-hangul에서 처리하지 않아도 된다고 생각했습니다!

@manudeli
Copy link
Member

@okinawaa conflict 해결부탁드려용

@okinawaa okinawaa requested review from manudeli and removed request for manudeli July 14, 2024 07:46
Copy link
Member

@manudeli manudeli left a comment

Choose a reason for hiding this comment

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

🚀

@okinawaa okinawaa merged commit fec20e8 into v2 Jul 14, 2024
9 of 10 checks passed
okinawaa added a commit that referenced this pull request Aug 4, 2024
* remove acronymize

* Create weak-walls-sniff.md

---------

Co-authored-by: Jonghyeon Ko <[email protected]>
okinawaa added a commit that referenced this pull request Aug 4, 2024
* remove acronymize

* Create weak-walls-sniff.md

---------

Co-authored-by: Jonghyeon Ko <[email protected]>
okinawaa added a commit that referenced this pull request Aug 5, 2024
* feat: 문자열에서 한글을 추출해주는 extractHangul 함수를 제거합니다 (#185)

* remove extrachHangul

* Create cyan-tigers-sneeze.md

---------

Co-authored-by: Jonghyeon Ko <[email protected]>

* feat: disassembleHangul, disassemble, disassembleHangulToGroup 함수에서 hangul이라는 글자를 제거합니다 (#184)

* dissemble관련 메서드에서 hangul이름을 제거합니다

* 누락된 부분 수정

* resolve conflit

* Create late-beers-hang.md

* diassembleHangul to diassemble

---------

Co-authored-by: Jonghyeon Ko <[email protected]>

* remove hangulIncludes (#188)

* feat: 한글의 두음을 반환해주는 acronymizeHangul 함수를 제거합니다. (#180)

* remove acronymize

* Create weak-walls-sniff.md

---------

Co-authored-by: Jonghyeon Ko <[email protected]>

* feat: getChoseng을 utils에서 별도 함수로 분리합니다. (#192)

* getChoseong분리

* write test code

* getChoseong import

* remove useless import statemenet

* remove unused file

* feat: `canBeChoseong`, `canBeJungseong`, `canBeJongseong` 을 utils에서 별도 함수로 분리합니다. (#193)

* canBe series를 별도 함수로 분리합니다

* add change set

* feat: `hasBatchim` 을 utils에서 별도 함수로 분리합니다. (#195)

* hasBatchim

* resolve type error

* add change set

* hasProperty, hasValueInReadOnlyStringList를 internal folder 로 옮깁니다

* resolve test error

* move with test code

* feat: choseongIncludes함수를 제거합니다. (#197)

* remove choseongIncludes

Co-authored-by: 서동휘 <[email protected]>

* fix: remove 한글

* fix: amountToMoneyCurrency로 함수명 변경

* fix: amountToHangul로 함수명 수정

* fix: conflict

* fix: filepath

* fix: import 방식 변경 및 CHANGELOG restore

* Create kind-birds-provide.md

---------

Co-authored-by: 박찬혁 <[email protected]>
Co-authored-by: Jonghyeon Ko <[email protected]>
Co-authored-by: 서동휘 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants