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

Markdownファイルのガイドライン適応 #1213

Merged
merged 10 commits into from
Feb 18, 2023

Conversation

nmori
Copy link
Contributor

@nmori nmori commented Feb 18, 2023

内容

markdownlint の検索範囲を広げるまえに、既存ファイルにガイドラインを適用します

関連 Issue

ref #1212

スクリーンショット・動画など

その他

  • 現状の仕組みではガイドラインを守れない点(文字長および imgタグのサイズ変更)は、例外処理としました
  • UIに関する部分もエラーを吐かないように直しましたが、 ヘルプ画面の見た目に多少影響があります
  • タグジャンプの関係で重複する見出しも修正しました

nmori added 9 commits February 5, 2023 11:35
・追加されたエンジンが未対応である場合には追加を阻止
・追加されてしまっている場合には、エラーで処理中断しないように
・MinimumEngineManifestの更新
・engineManifests[selectedId]自体が undefined であるケースに対応
- markdownlintの適用にあたって現状起きているエラーを修正(ref VOICEVOX#1212)
- FIXME:imgのサイズ変更指示、メッセージ長超過が規則違反なのでやむを得ず例外対応(/.markdownlint.json)
@nmori nmori requested a review from a team as a code owner February 18, 2023 04:35
@nmori nmori requested review from Hiroshiba and removed request for a team February 18, 2023 04:35
@nmori
Copy link
Contributor Author

nmori commented Feb 18, 2023

一通り修正を実施しました。

なお、Markdownの修正がおわったら、
最後にtestシーケンスを下記のように適用すれば完了かとおもっています。

      - name: Check Markdown
        run: npm run markdownlint ./*/*.md

@Hiroshiba
Copy link
Member

修正ありがとうございます!!!! すごく助かります!!

npm run markdownlint .//.md

こちらいろいろ検証した感じ、package.jsonのmarkdownlintのところをこう変えてからnpm run markdownlintする形が良いかなと思いました!!

"markdownlint": "markdownlint --ignore node_modules/ --ignore dist/ --ignore dist_electron/ ./",

./*/*.mdだとnode_modelesの下も見ちゃうので。。)

@nmori
Copy link
Contributor Author

nmori commented Feb 18, 2023

・ちなみに、line-length は、 ALTタグが引っかかるので、無視項目として追加しています。
・お問い合わせは、VOICEVOX左側のメニューがそうなっているため、そのH1タグにしました。
 (ルール上H1タグが必要という事でしたので追記しています)

public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
@nmori
Copy link
Contributor Author

nmori commented Feb 18, 2023

・今回は80文字制限をignoreにしているものの、極力制約にひっかからないように変更しています。
 わかりやすさの件、理解しましたので修正・戻しいれていきます。ありがとうございます。

public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/howtouse.md Outdated Show resolved Hide resolved
public/qAndA.md Outdated Show resolved Hide resolved
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!

ところどころ文面的に微妙だなと思っていたところも良い感じにして頂けて助かりました!!
ありがとうございます!!

@Hiroshiba
Copy link
Member

Markdownのテストに関して、npm run markdownlintで大丈夫になったため既存のこちらのテストが働きそうです!

- run: npm run markdownlint

ということで大丈夫だと思うのでマージします!!

@Hiroshiba Hiroshiba merged commit f1b98d6 into VOICEVOX:main Feb 18, 2023
@nmori nmori deleted the markdownの修正#1212 branch February 19, 2023 07:43
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.

2 participants