-
Notifications
You must be signed in to change notification settings - Fork 180
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: Update lint:text command for incoming document #454
Conversation
動作の確認として以下の事項をテストし、問題がないことを確認しました。
|
beta ディレクトリの中はリポジトリのトップレベルとは独立した package.json や yarn.lock を持つ別環境であり、beta ディレクトリの中と外の世界(現行サイト)が互いに干渉しないことになっているように見えます。変則的な monorepo と言いますか。現行サイトは Gatsby で新サイトは Next.js ですし。 将来的には beta の中身がそっくりそのままリポジトリのルートを置き換え、現行のリポジトリルートにある(beta 以外の)内容はどこかにアーカイブされます。逆に、現在のリポジトリのルートにあるコンテンツやスクリプトはあくまで現行のドキュメントサイトを担当しており、betaディレクトリの中身について一切関知してはいけません。 このPRでは、現行サイトを構築するためのスクリプトが beta の存在に依存してしまっていますし、将来 beta がメインサイトとして昇格する際にも問題が生じてしまいます。面倒でも、beta という別環境に、現行の textlint と同様のセットアップを再現する必要があると思います。 |
言い方を変えると、クローンした後に beta ディレクトリだけ全く別の場所に動かしたとしても、現行サイトは beta なしで完璧にビルドできる必要がありますし、beta は beta で親フォルダにあったファイルへの依存なしに完璧にビルドできる必要があります。 |
レビューありがとうございます!たしかにこのPRのままでは移行時に困りますね。 Betaディレクトリで独立して運用できるように、betaのpackage.jsonにtextlintとrule-presetを追加し、設定ファイルをコピーしようと思います。 |
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.
普段は、lint-staged という NPM パッケージを利用して、コミット前のフックでコミット直前に当該ファイルのみに textlint を走らせています。それについては #126 をご覧ください。
#126 相当のものもここで含めてからマージできれば理想なのですが、いかがでしょうか。大変そうならそちらは別PRでやるのでも大丈夫です。
beta 側では既に husky が使われていますので、そちらの別途インストールは不要です。lint-staged を使って、既存の husky のセットアップを調整する形になります。
あと本家側でもコミット前に全部 prettier や eslint をかけるのではなく lint-staged を使ったらどうなの、と私がもちかけており(reactjs/react.dev#4143)、タイミングによりそちらとの干渉がありえることにも留意ください。
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.
1カ所だけ細かいですが修正お願いします。
(ちょっと自分の環境固有っぽい問題で動作確認がとれていないのでapproveは少しだけお待ちください)
うちの環境でこの PR をチェックアウトし、 以下の Issue をヒントに調べたところ、どうも もうちょっと調べてみます… |
https://stackoverflow.com/a/56026616/1209240 どうも pre-commit フック内で cd しようとすると面倒なよう。 どうせ本家側で lint-staged いれた時にまた考え直すことになるので、とりあえず日本版でうまく動くこと優先でいいかなと思います |
自分の環境ではcommitできてます。 とりあえず動くようにすることで良いと思いますが、この修正を@smikitkyさんにお願いできますか? |
@no-yan すみません、平日はなかなか時間がとれていないのですが "Allow edits from maintainers" というのはチェックされていますでしょうか。一度 push を試そうとしたら no write access と言われたので。 また、そちらの環境での |
懸念だった reactjs/react.dev#4139 これが upstream でマージされ、これで目に見える問題は一通り解決しそうなので、これをこのリポジトリにマージするときに改めて見直したいと思います。遅くなり申し訳ありません。 |
一からPRを作ったほうが早いと思われる場合は、どなたでも躊躇なくPRを出していただいて構いません。 |
@no-yan お気遣いありがとうございます。そろそろ翻訳進めていかないとなーと思って手を付けられていないというだけです。仰る通り、これをベースにするより現状の最新からやりなおした方が早いとなったらそうさせていただくかもしれませんが、その場合はよろしくお願いします。 |
ごめんなさい、リポジトリ構成がとんでもなく変わったので、textlint のセットアップも全体的にゼロからやりなおすことにしました。こちらは close させていただきます。 新しい PR は #580 です。 |
This PR is a part of #453.
概要
Textlintを実行する
lint:text
コマンドがbeta/src/pagesフォルダもlintするよう変更しました。また、以下の変更を加えました。
globパターンをvalidにlintされていないディレクトリが存在したため(content/home/{examples, marketing})、これを修正しました。TextLintではglobパターンを引用符で囲わなかった場合、Textlintはサブディレクトリの探索を行いません。修正方法はtextlintのindex.mdに従いました。cacheの有効化エラーを懸念してやらないことに決定しました。
lint対象が増えtextlintの処理時間が長くなることが予想されたので、--cacheオプションを有効化し高速化しました。手元の計測では12.8s => 2.3sと5.5倍高速です。note:
MDX対応は行いませんでした。これは新ドキュメントの拡張子もすべて
.md
で必要なかったためです。textlint-plugin-markdownで
.mdx
拡張子をlintすることは可能ですが、これは.mdと同じ方法でlintしているだけです。また、textlintのmdxルールセットは見つかりませんでした。