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

chore: version packages #232

Merged
merged 1 commit into from
Aug 12, 2024
Merged

chore: version packages #232

merged 1 commit into from
Aug 12, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 9, 2024

This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.

Releases

es-hangul@2.1.0

Minor Changes

  • #228 0633b9f Thanks @BO-LIKE-CHICKEN! - feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가

Copy link

vercel bot commented Aug 9, 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 Aug 9, 2024 5:05pm

@manudeli
Copy link
Member

manudeli commented Aug 9, 2024

#228 배포전에 public api면 src/*에 위치시키면 어떨까요? 다른 public api들은 그 위치에 있는 거 같고 배포전이라 멘션드립니다. @BO-LIKE-CHICKEN @okinawaa

src/days.spec.ts, src/days.ts(상수들도 여기에 위치시켜서 constants 파일도 하나로 합칠 수 있을 거 같아요. 다른 파일에서 접근하면 안되는 파일이라 exports하지 않으면 좋겠어요) 이렇게 어떨까요?

dates라는 이름의 폴더로 아직 묶을 필요가 없어보였어서 1:1로 매칭되게 하면 일관되게 할 수 있을 거 같습니다

@BO-LIKE-CHICKEN
Copy link
Contributor

기존에는 src/* 에 위치시켰지만, constants 파일을 분리하는 피드백을 반영하며 standardizePronunciation를 참고해 이 커밋에서 폴더구조를 바꿔보았는데요. 🤔

다른 파일에서 접근하면 안되는 파일이라 exports하지 않으면 좋겠어요) 이렇게 어떨까요?

에 대한 부분이 염려된다는 점과, 더 일관성 있는 구조를 위해 src/*에 위치시키는 것에 동의해요!

다만, 저는 days는 분명하게 날짜라는 관심사에 포함된 함수라고 생각이 들어서 아래와 같은 구조와 이름이라면 좋겠어요.

── src
│  ├── date.ts
│  ├── date.spec.ts
│  └── constants.ts (days.constants.ts 가 합쳐져요)

비록 아직 days라는 함수 하나만을 제공하긴 하지만, 날짜라는 같은 관심사를 같는 함수들이 추가될 여지가 확실하기에 es-hangul문서date에 준비 중인 함수를 안내한다던가, date에 없는 기능을 기여하고 싶게끔 할수도 있다는 생각이 들어요!

그럼에도 아직은 days로 관리하는 편이 좋을 것 같다는 의견이시라면 말씀부탁드려요!

누군가 구조를 잡을때 이 PR이 도움이 될 수 있을 것 같네요 😁

@manudeli
Copy link
Member

manudeli commented Aug 10, 2024

기존에는 src/* 에 위치시켰지만, constants 파일을 분리하는 피드백을 반영하며 standardizePronunciation를 참고해 이 커밋에서 폴더구조를 바꿔보았는데요. 🤔

다른 파일에서 접근하면 안되는 파일이라 exports하지 않으면 좋겠어요) 이렇게 어떨까요?

에 대한 부분이 염려된다는 점과, 더 일관성 있는 구조를 위해 src/*에 위치시키는 것에 동의해요!

다만, 저는 days는 분명하게 날짜라는 관심사에 포함된 함수라고 생각이 들어서 아래와 같은 구조와 이름이라면 좋겠어요.

── src
│  ├── date.ts
│  ├── date.spec.ts
│  └── constants.ts (days.constants.ts 가 합쳐져요)

비록 아직 days라는 함수 하나만을 제공하긴 하지만, 날짜라는 같은 관심사를 같는 함수들이 추가될 여지가 확실하기에 es-hangul문서date에 준비 중인 함수를 안내한다던가, date에 없는 기능을 기여하고 싶게끔 할수도 있다는 생각이 들어요!

그럼에도 아직은 days로 관리하는 편이 좋을 것 같다는 의견이시라면 말씀부탁드려요!

누군가 구조를 잡을때 이 PR이 도움이 될 수 있을 것 같네요 😁

현재도 이미 canBe라는 이름으로 묶여있지만 이 인터페이스도 1:1 매칭이 된다면 일관되기 때문에 더 이해하기 쉬울 거 같아요. date도 이것이 인터페이스의 이름인지 카테고리인지 알기어려운 이유가 다른 것들은 카테고리화되어있지 않기 때문에 위계가 달라서 인 거 같아요. 그래서 같은 위계라면 같은 역할이었으면 하는 바람이 있습니다. 현재는 date카테고리가 생기더라도 들어가야하는 게 1개밖에 없는 상황에서 다른 인터페이스가 들어갈 것이 아니라면 왜 date가 카테고리화되어야하는지는 아직은 이해되지 않는 것 같아요

  1. Date에 들어갈 다른 인터페이스가 무엇인지 명확히 하는 것
  2. 다른 인터페이스들을 같은 위계로 정리하는 것

아래는 탠스택쿼리의 api references 섹션인데 1:1로 되어있어서 들어가보지 않아도 한 번에 이해할 수 있는 점이 좋았던 거 같아서 스크린샷 남겨보아요

Screenshot_20240810_124551_Chrome

저도 이게 정답이 있는 내용은 아니라고 생각해서 @okinawaa 가 결정해주시면 좋겠어요

@BO-LIKE-CHICKEN
Copy link
Contributor

BO-LIKE-CHICKEN commented Aug 10, 2024

@manudeli 님 상세한 의견과 좋은 레퍼런스까지 남겨 주셔서 감사해요! 이야기를 들어보고 저도 다음과 같은 대목이 가장 공감되었어요.

다른 것들은 카테고리화되어있지 않기 때문에 위계가 달라서 인 거 같아요. 그래서 같은 위계라면 같은 역할이었으면 하는 바람이 있습니다.

하여, src/*에 위치시켜두었던 카테고리 역할을 하며 api를 제공하는 date.tsdays.ts로 바꾸고, 동시에 canBe도 말씀하신 것처럼 1:1로 매칭시켜 같은 위계에 있는 파일들은 같은 역할을 하도록 바꾸면 어떨까요? 🙂

지금은 말씀하신 것처럼 같은 위계에서 다른 역할을 하는 파일들이 있어서 이후 다른 기능들이 추가될때도 혼란스러울 것 같아요

그리고 나중에 "제공하는 api나 기능이 너무 많아서 문서에서 찾기가 어려워요" 라는 공감대가 형성된다면 API들을 어떻게 그룹화 할 것인지를 논의해보면 어떨까요? (문서에서만 그룹화하거나 카테고리를 나눌지, 다른 구조를 가져갈지)

저도 이 부분에 정답은 없다고 생각하고 다만, 합의된 규칙만 있다면 좋겠다는 생각입니다.

저도 이 패키지를 관리하는 분들에게 판단을 맡길게요!

@manudeli manudeli requested review from okinawaa August 10, 2024 12:34
@okinawaa
Copy link
Member

두분 값진 논의해주셔서 감사합니다
현재는 함수의 종류도 크게 많지 않아서 헷갈릴 여지가 없을 것 같아요.
점점 새로운 기능을 제공해주는 함수의 종류들이 많아지고 그에 따라 grouping이 가시화 될 것 같아요.
내부 폴더구조만 변하는 정도라, Breaking Change는 아니라고 생각하여 그때 논의해봐도 좋을 것 같습니다.
다시 한번 감사합니다 🙇

@okinawaa okinawaa merged commit 4a2660a into main Aug 12, 2024
9 checks passed
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.

3 participants