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

docs: susa 함수에 jsdoc을 추가합니다. #250

Merged
merged 4 commits into from
Sep 15, 2024

Conversation

minsoo-web
Copy link
Member

@minsoo-web minsoo-web commented Sep 15, 2024

Overview

susa에 jsdoc을 추가합니다.

#201 (comment) 에서 논의된 내용을 바탕으로 만들어졌어요.
Special Thanks to @BO-LIKE-CHICKEN

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.

Copy link

changeset-bot bot commented Sep 15, 2024

🦋 Changeset detected

Latest commit: 56e8705

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

Copy link

vercel bot commented Sep 15, 2024

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

Name Status Preview Comments Updated (UTC)
es-hangul ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 2:50pm

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.66%. Comparing base (f3e9024) to head (56e8705).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #250   +/-   ##
=======================================
  Coverage   99.66%   99.66%           
=======================================
  Files          36       36           
  Lines         601      601           
  Branches      145      145           
=======================================
  Hits          599      599           
  Misses          2        2           

src/susa/susa.ts Outdated Show resolved Hide resolved
Co-authored-by: Bosung Baek <[email protected]>
Copy link
Member Author

@minsoo-web minsoo-web left a comment

Choose a reason for hiding this comment

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

고민되는 지점을 공유해요!

Comment on lines +22 to +24
*
* @see https://es-hangul.slash.page/docs/api/susa
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

@BO-LIKE-CHICKEN 안녕하세요, 고민되는 지점이 있어서 멘션드리게 되었어요
제가 수사랑 수관형사가 뭔지 잘 몰라서, 결국 수사가 뭔지 수관형사가 무엇인지 검색해볼 것 같다는 생각이 들더라구요
@see 수사? https://ko.dict.naver.com/#/entry/koko/d0ce2b674cae4b44b9028f648dd458b0

이런 링크도 첨부하는 것이 좋을지 고민됩니다!
cc @okinawaa

Copy link
Contributor

Choose a reason for hiding this comment

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

링크를 제공해주는 것은 너무 좋은 친절함 같지만, 링크를 전달하는 방식에서 이런 아이디어를 제안드려요 🙇🏻‍♂️

  1. @see 태그에서는 es-hangul에서 제공하는 docs로의 링크만 제공하는 것이 좋을 것 같아요
  2. "한글"이라는 복잡한 도메인의 특성상 앞으로도 "한국인도 어려운 도메인 지식"을 만나게 될텐데 다음과 같은 방식으로 제공해주면 어떨까해요

이렇게 제공하게되면 어려운 한글 도메인에 대한 설명 뿐만 아니라 의문을 가질 수 있는 함수의 제한(왜 1~100 까지만 지원하지?)에 대해서도 설명해 줄 수 있을 것 같아요 🙂

Screenshot 2024-09-15 at 9 31 45 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

감사합니다 !!! 너무 좋네요!!! 🙇

src/susa/susa.ts Outdated Show resolved Hide resolved
Co-authored-by: Bosung Baek <[email protected]>
Copy link
Member

@okinawaa okinawaa 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 c0ca9f5 into toss:main Sep 15, 2024
8 of 10 checks passed
@github-actions github-actions bot mentioned this pull request Sep 15, 2024
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