-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Monorepo 지원 및 전반적인 재조정 #82
Conversation
주말에 소중한 시간을 내주시고, 멋진 모노레포 작업에 진심으로 감사드립니다! 큰 변경사항이어서, 다른 분들과 리뷰를 진행하고 알려드리겠습니다! |
따로 문제되는 부분은 없으나, readme상에 링크가 잘못되어 있어(변경된 경로구조를 적용하지 않음.) 수정하도록 하겠습니다. |
jsdelivr의 링크 적용 방법은 두가지입니다.
아무래도 깃허브로 부터 받아오면 링크도 길어지고 하여 npm 기준으로 받아오는게 좋을 거 같긴합니다만, npm에 퍼블리시 될 동안은 사용할 수 없다는게 단점입니다. |
현재는 push를 github로 부터 가져오는 gh path를 적용합니다만, 이 PR에서 해당 부분 이야기 한 뒤에 적용하는 게 좋을 거 같아요. |
black7375/pretendard#1 에 PR 드렸습니다. |
jsdelivr 말고 cdnjs나 unkpg에도 영향이 있나 확인해보는게 좋을 것 같습니다. |
조금 고민이 되는 것이, 이미 기존에 GitHub 기준으로 jsDelivr로 제공되던 CSS 주소들이 호환이 되지 않을 것 같습니다. 이 경우 기존에 Pretendard 웹폰트를 사용하시는 분들께 오류가 발생할 것으로 보입니다. 아래 2가지 방법이 있을 것 같은데... 제 생각이 짧아 한 번 검토가 필요합니다.
|
모노레포 구성에 관해서는 제가 깔끔하게 알지 못해 여쭤보기 조심스럽긴 한데요, 저도 hiddenest님께서 첫 번째로 알려주신 것처럼 기존 cdn url과의 호환성을 위해 dist 폴더 파일 및 구조를 유지하는 방향은 어떤지 궁금합니다. 제가 생각한 방식은 GitHub Actions를 통해 각 패키지에 맞지 않은 파일들은 제외해 publish하는 것이었는데요, 비슷하게 dist를 유지한 채로 모노레포를 관리할 수 있는 방법이 있나요? 👀 |
제 생각에는 root/dist에 font-face의 url 경로만 바꾸어 저장하면 어떨까 싶어요. 아니면 살짝 무책임하지만 import문으로 로드하고, 성능에 문제가 생길 수 있다는 고지와 함께 Deprecated 처리해버리는 것도..? |
@black7375 네, 저도 말씀해주신 방법도 나쁘지 않을 것 같습니다. (font-face의 url 경로만 바꾸어 저장) 이전에 있었던 jsdelivr 등의 캐시 관련 문제 때문에(#68 (comment)), 이 부분에 같이 고려되어야 하는지 확인이 필요할 것 같습니다. |
버전을 강제할 수 있다면 베스트일 것 같아요. |
빠르게 의견을 남겨주셔서 감사합니다! Merge 이후에 기존 CDN URL 업데이트가 필요하다는 공지가 필요하겠네요. 또다른 사이드 이펙트는 없는지 확인해보고 다시 알려드리겠습니다! |
잠시 시간이 나서 CDN 호환성을 유지하며 업데이트하는 방법을 생각해봤어요. 1. jsdelivr 버저닝과 캐싱앞서 이야기했듯 버저닝을 도입하는것이 캐싱 문제를 해결하는데 도움이 됩니다. <link rel="stylesheet" as="style" crossorigin href="https://cdn.jsdelivr.net/gh/orioncactus/pretendard/dist/web/static/pretendard.css" /> 다음과 같이 업데이트될 겁니다.
<link rel="stylesheet" as="style" crossorigin href="https://cdn.jsdelivr.net/gh/orioncactus/[email protected]/dist/web/static/pretendard.css" /> 2. 구조변경과 호환성 유지구조가 변경되어도 예전 URL을 그대로 사용가능해야 합니다. 현재로선 CSS 파일이 업데이트 될 때마다 실행하면 됩니다. |
@black7375님! 답장이 늦어 죄송합니다. 여러 개발자분들과 모노레포 작업이 Pretendard의 현상황 및 향후 방향성에 적합한지, 그리고 다른 글꼴 프로젝트는 어떻게 관리하는지 확인하는 데 시간이 조금 걸렸습니다. 정리하면 다음과 같습니다.
3번이 가장 큰 고민이어서 답장이 늦었는데요, npm에서 publish 가능한 package 최대 용량은 256MB고(확인해주신 @victorrica님께 감사드립니다!), npm, Yarn에서 Pretendard를 설치할 때 대부분 사용하지 않을 Pretendard JP와 Pretendard Std까지 설치되는 게 적합하지 않은 듯해 모노레포 구성을 제안드렸으나, 비슷하게 다양한 패밀리로 제공 중인 IBM Plex에서는 모노레포로 패키지를 구성하지 않아 혹시나 제가 잘못된 판단을 하고 있는 것은 아닌지, 또 혹시나 기여해주신 소중한 시간을 날려버리는 것은 아닌지 알아보는 데 시간을 많이 썼습니다 😅 정리하면, 제안해주신 방향은 향후 Pretendard 프로젝트를 관리할 때 큰 도움이 될 듯합니다. 다만 Pretendard 프로젝트가 점점 많은 분들이 유용하게 써주시면서, 이러한 의사결정을 제가 단독으로 결정하기에는 혹여나 문제 여지는 없을까 싶어서, 여러분들의 리뷰가 절실합니다. 크게 이견이 없다면 이번주 주말에 merge할 수 있을 듯합니다. 그동안 저도 마지막으로 꼼꼼하게 확인해보고, 빠르게 적용할 수 있도록 힘쓰겠습니다. 소중한 시간을 Pretendard 기여에 힘써주셔서 진심으로 감사드립니다! 궁금합니다!그리고 궁금한 점인데요, Pretendard 레포지터리 구조가 많이 바뀌었으므로 이 PR을 적용한 뒤에 새 버전인 1.3.4로 배포하는 것이 좋을지, 아니면 이후 글꼴 업데이트 때 버전을 올리는 게 좋을지 궁금합니다 👀 |
Co-authored-by: alstjr7375 <[email protected]> Co-authored-by: MacBook Air M1 <[email protected]>
좋은 피드백 해주셔서 감사합니다. 현재 Npm 업로드 문제가 있고, 패키지가 쪼개지는거라 |
확인했습니다! 그렇다면 모든 작업이 정리되면 merge한 다음 버전도 올려두겠습니다! 🔥 소중한 시간을 내주셔서 다시 한번 감사드립니다! |
어짜피 버저닝을 붙일거고, 모노레포로 만든 김에 url을 npm 기준으로 만드는게 주소도 짧고 관리하기 나을거 같습니다.
|
@black7375 @orioncactus 변경되는 김에, 구글 폰트에 맞게 폴더를 이동하려고 합니다. |
black7375/pretendard#3 PR 드렸습니다. 테스트 후 머지 부탁드립니다. |
저도 고려했었는데, 최신 버전의 경우 NPM 업로드 문제 때문에 링크가 작동하지 않습니다. (1.2.2가 최신인 상태)
역시 문제가 호환성인데요, [기타 구글 폰트 등록에 있어서] 일단 프리텐다드의 글꼴빌드에 문제가 있어 근시일 내에 등록하기 힘들 수도 있습니다. 구글폰트의 레포를 보면 또 아주 엄격하게 따르는 것 같지는 않기도 하고, 모노레포다보니 변경이 얼마나 필요한지 조사가 필요할 것 같습니다. |
|
@kms0219kms 소중한 시간을 Pretendard에 기여해주시고 의견을 남겨주셔서 감사드립니다! 제안해주신 jsDelivr 경로는 기존에 사용하던 GitHub를 통한 url로 그대로 유지하는 것이 일관성 면에서 좋을 듯합니다. 기존에 사용하던 코드에서 그리고 @black7375님께서 말씀해주신 것처럼, Google Fonts에서는 가이드라인에 따르지 않은 경우도 적지 않게 있어서, 제가 직접 커뮤니케이션해보면 가능하지 않을까 싶습니다. 추후에 Google Fonts에 등재할 때 디렉터리 구조로 이슈가 있다면 그때 진행해도 괜찮을 듯합니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@black7375님! 우선 문서 경로와 약 3,000개 정도 되는 폰트 에셋들을 제외한 나머지를 먼저 확인했는데요, 경로와 폰트 에셋들은 제가 마지막에 훑어보려고 합니다. 혹시나 제가 아래에 확인한 내용 말고도 놓친 부분이 있다면 알려주시면 감사하겠습니다! 준비가 다 되어 알려주시면, 마지막으로 다른 분들과도 이상이 없는지 확인하고, GitHub Actions가 잘 작동하는지 확인한 다음 머지하려고 합니다 🔥
Readme 업데이트 관련
- 버저닝 관련: @kms0219kms님께서 버전을
v1.3.3
과 같이 추가해주신 것을 확인했습니다. 크게 이견이 없어 그대로 반영하려고 합니다. 업데이트해주셔서 감사드립니다! - css 포매팅 관련: @kms0219kms님, 기존에 readme에서 제공하던 css 코드에서 작은따옴표(
'
)를 큰따옴표("
)로 바꾸신 것에 대한 궁금점인데요, css에서도 html과 마찬가지로"
와 같이 쓰는 것이 더욱 바람직한가요? 잘 몰라 여쭤봅니다! - 번역 문서 바로가기 관련: Pretendard JP, Pretendard Std 문서에서 누락된 바로가기를 추가해주신 것을 확인했습니다. 꼼꼼하게 확인해주시고 업데이트해주셔서 감사드립니다!
- Pretendard를 사용하는 곳 관련: @kms0219kms님께서 청와대, 국민 폼으로 웹사이트를 추가해주신 것 확인했습니다! 확인해주시고 추가해주셔서 감사드립니다! 😄
모노레포 마이그레이션 및 서브셋 자동화 관련
엄청난 작업임에도 불구하고 리팩토링까지 멋지게 진행해주셔서 다시 한번 진심으로 감사드립니다!
GitHub Actions 관련
혹시 작업 중인 게 있으시다면 알려주시면 감사하겠습니다! 패키지 업로드를 제외하고 나머지가 잘 작동하는지 테스트 레포지터리에서 확인해보려고 합니다.
NPM_TOKEN
도 Automation Type로 GitHub Actions Secret으로 추가해두었는데, Type이 Automation이 아닌 Publish로 해야 한다면 알려주시면 감사하겠습니다!
packages/*/dist에 LICENSE.txt 관련
pretendard-jp와 pretendard-std에 LICENSE.txt가 보이지 않아서, 이건 제가 merge 이후 추가하겠습니다!
지금 액션에서 1~2개 문제가 있는것 같아서 테스트 중이에요. CSS의
라이센스는 원래 프로젝트 루트에 두고, 릴리즈시 복사하는 방식을 사용하려고 했는데, |
액션을 실행해본 결과 문제가 있던 점을 수정했습니다. 폰트 생성
릴리즈
이번 풀리퀘스트는 변경량이 많기 때문에 내역을 추적이 가능하도록 |
@black7375님! 소중한 주말에 확인해주시고 신경써주셔서 정말 감사드립니다.
이제 거의 마무리된 것 같아요. 머지 이후 릴리즈와 크레딧에 기여해주신 공로를 남겨두려고 합니다. 엄청난 작업임에도 이렇게 도와주셔서 다시 한번 감사드립니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인해보니 distCSS.sh로 생성된 dist/web에 있는 css에서 url path가 잘못되어 폰트가 불러와지지 않는 듯합니다. 또 제가 distCSS.sh를 실행해보니 dist에 들어가는 css에서 url path가 수정이 안 된 상태로 적용되어서 /distCSS.sh
파일 업데이트가 필요할 것 같은데요, 우선 저도 바로잡을 수 있는지 확인해보겠습니다.
- 문제 파일:
/distCSS.sh
- 문제: dist/web에 있는 css에서 에셋 경로가
../packages
와 같이 구성됨 - 업데이트:
../packages
와 같은 경로를../../../packages
와 같이 수정
헉 이렇게 기본적인 실수를ㅠㅠ |
테스트 레포지터리로도 돌려보고 알려드려요. distCSS.sh 업데이트 관련늦은 밤이었는데 빠르게 수정해주셔서 감사합니다! 다만 리눅스가 아닌 macOS에서는 해당 스크립트가 작동이 안되는 것 같은데요, 방법을 생각해보았으나 어차피 각 packages에 포함된 css가 업데이트되면 dist에 있는 css도 수정되어야 하기 때문에, 아예 리눅스로 구동되는 GitHub Actions에 붙여넣어보려고 합니다. 제가 해본 방식은 다음과 같습니다:
다이나믹 서브셋을 제외한 에셋 업데이트 관련GitHub Actions로 돌려보니 다이나믹 서브셋을 제외한 웹폰트 에셋이 업데이트되어 혹시나 이슈가 있나 각 브라우저에서 확인해보았는데, 우선은 모두 문제가 없어 보입니다. 제가 테스트해본 macOS가 아닌 Windows에서도 이슈가 없는지도 확인해보겠습니다. packages에 포함된 css 관련이것도 문제 없이 잘 표시되었습니다! css
|
말씀 주신대로 워크플로우에 통합했습니다. |
제가 아직 중학생인터라 시험 준비하느라고 접속을 잘 못했네요. 별다른 이슈가 없으면 이렇게 진행하시면 됩니다. 머지하시고, npm에 퍼블리시 해두시면 시험 끝나고 cdnjs에 등록 요청 넘기겠습니다. Google Fonts 경로 문제도 해결되는대로 알려주시면, PR 넘겨 두겠습니다! |
바쁜기간에 시간을 내 제 작업을 검토해주셔서 감사합니다 👍 |
Windows에서도 정상 작동 확인하고, 새 otf 및 ttf 에셋으로 GitHub Actions를 돌릴 때에도 모두 문제 없이 작동되는 것을 확인했습니다. merge 이전에 추가적으로 확인이 필요한 게 있다면 알려주시면 감사하겠습니다. 확인이 필요한 게 딱히 없다면 내일 밤 중 마지막까지 기여해주시고 신경써주신 @black7375님, @kms0219kms님(시험 응원합니다! 🔥), @hiddenest님 모두 진심으로 감사드립니다! 기여자 목록 업데이트 관련이제 마지막으로 기존에 썼던 기여자 목록 내용을 업데이트하려고 하는데요, 다음과 같이 쓰면 될지, 혹시나 다듬었으면 하는 내용이 있다면 알려주시면 감사하겠습니다!
merge 이후 기존 jsDelivr URL에 버전 추가 안내 관련merge 이후에 안정적인 사용을 위해 기존에 사용 중인 CDN URL에 버전을 추가해달라는 내용을 공지하면 좋을 것 같은데요, 우선 제 소셜 미디어 계정으로 진행하려고 하는데, 혹시나 공유하기 좋은 커뮤니티가 있다면 알려주시면 감사하겠습니다! |
기여자 업데이트버저닝, 문서 포매팅 관련해 저랑 @kms0219kms 의 내역을 바꾸어 적어주시면 될 것 같아요. black7375#1 홍보채널제가 아는걸론 긱뉴스가 꽤 유명하고, Pretendard에 대한 반응이 좋습니다!! |
@black7375님, 빠르게 확인해주셔서 정말 감사드립니다! 기여자 관련확인했습니다! 그렇다면 다음과 같이 업데이트하겠습니다. 혹시나 맞지 않는 부분이 있다면 알려주시면 감사하겠습니다!
홍보 채널 관련GeekNews에서 직접 올려도 괜찮은지 확인해보고, 괜찮다면 올려보겠습니다. 알려주셔서 감사합니다! |
모노레포 제안에 흔쾌히 기여해주시고 오래 기다려주신 @black7375님, 해당 Pull Request를 확인해주시고 개선해주신 @kms0219kms님, 더 나은 방향성을 위해 피드백을 남겨주신 @hiddenest님께 모두 진심으로 감사드립니다! 덕분에 더욱 효율적으로 Pretendard를 관리할 수 있을 듯합니다. 적용되었습니다! ᕙ(•̀‸•́‶)ᕗ |
드디어 merge가 되었군요! |
관련 이슈:
주의!!!
변경사항이 매우 많아 리뷰가 꼭 필요합니다.
나름 꼼꼼히 살폈다고 하나 바뀐점이 많다보니 실수한게 있을수도 있습니다.
주요 변경사항
packages/*
형식으로 변경dist/*
에 있던 폰트/CSS파일들과docs/*
에 있던 문서들은 각각packages/폰트/dist
,packages/폰트/docs
위치로 rename 처리PretendardJP
와PretendardStd
의 헤더에 언어변경 링크 추가subset-utils
패키지 빌드 요구yarn npm publish --access public
실행 (주의할 변경사항)NPM_TOKEN
로 깃허브 시크릿을 설정해주셔야 정상적으로 작동가능합니다.